On 06/10/20 15:18, Douglas Anderson wrote: > The cros_ec_spi driver is realtime priority so that it doesn't get > preempted by other taks while it's talking to the EC but overall it > really doesn't need lots of compute power. Unfortunately, by default, > the kernel assumes that all realtime tasks should cause the cpufreq to > jump to max and burn through power to get things done as quickly as > possible. That's just not the correct behavior for cros_ec_spi. > > Switch to manually overriding the default. > > This won't help us if our work moves over to the SPI pump thread but > that's not the common code path. > > Signed-off-by: Douglas Anderson <diand...@chromium.org> > --- > NOTE: This would cause a conflict if the patch > https://lore.kernel.org/r/20200422112831.870192...@infradead.org lands > first > > drivers/platform/chrome/cros_ec_spi.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_spi.c > b/drivers/platform/chrome/cros_ec_spi.c > index debea5c4c829..76d59d5e7efd 100644 > --- a/drivers/platform/chrome/cros_ec_spi.c > +++ b/drivers/platform/chrome/cros_ec_spi.c > @@ -709,8 +709,11 @@ static void cros_ec_spi_high_pri_release(void *worker) > static int cros_ec_spi_devm_high_pri_alloc(struct device *dev, > struct cros_ec_spi *ec_spi) > { > - struct sched_param sched_priority = { > - .sched_priority = MAX_RT_PRIO / 2, > + struct sched_attr sched_attr = { > + .sched_policy = SCHED_FIFO, > + .sched_priority = MAX_RT_PRIO / 2, > + .sched_flags = SCHED_FLAG_UTIL_CLAMP_MIN, > + .sched_util_min = 0,
Hmm I thought Peter already removed all users that change RT priority directly. https://lore.kernel.org/lkml/20200422112719.826676...@infradead.org/ > }; > int err; > > @@ -728,8 +731,7 @@ static int cros_ec_spi_devm_high_pri_alloc(struct device > *dev, > if (err) > return err; > > - err = sched_setscheduler_nocheck(ec_spi->high_pri_worker->task, > - SCHED_FIFO, &sched_priority); > + err = sched_setattr_nocheck(ec_spi->high_pri_worker->task, &sched_attr); > if (err) > dev_err(dev, "Can't set cros_ec high pri priority: %d\n", err); > return err; If I read the code correctly, if you fail here cros_ec_spi_probe() will fail too and the whole thing will not be loaded. If you compile without uclamp then sched_setattr_nocheck() will always fail with -EOPNOTSUPP. Why can't you manage the priority and boost value of such tasks via your init scripts instead? I have to admit I need to think about whether it makes sense to have a generic API that allows drivers to opt-out of the default boosting behavior for their RT tasks. Thanks -- Qais Yousef > -- > 2.27.0.290.gba653c62da-goog >
