Messages in this thread | | | Date | Thu, 23 Apr 2020 10:46:26 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 04/23] sched,acpi_pad: Convert to sched_set_fifo*() |
| |
On Wed, Apr 22, 2020 at 06:45:36PM +0200, Dietmar Eggemann wrote: > On 22/04/2020 13:27, Peter Zijlstra wrote: > > Because SCHED_FIFO is a broken scheduler model (see previous patches) > > take away the priority field, the kernel can't possibly make an > > informed decision. > > > > In this case, use fifo_low, because it only cares about being above > > SCHED_NORMAL. Effectively no change in behaviour. > > > > XXX: this driver is still complete crap; why isn't it using proper > > idle injection or at the very least play_idle() ? > > > > Cc: rafael.j.wysocki@intel.com > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Reviewed-by: Ingo Molnar <mingo@kernel.org> > > --- > > drivers/acpi/acpi_pad.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > --- a/drivers/acpi/acpi_pad.c > > +++ b/drivers/acpi/acpi_pad.c > > @@ -136,12 +136,11 @@ static unsigned int idle_pct = 5; /* per > > static unsigned int round_robin_time = 1; /* second */ > > static int power_saving_thread(void *data) > > { > > - struct sched_param param = {.sched_priority = 1}; > > int do_sleep; > > unsigned int tsk_index = (unsigned long)data; > > u64 last_jiffies = 0; > > > > - sched_setscheduler(current, SCHED_RR, ¶m); > > I was wondering what happened to the SCHED_RR cases but as I can see now > they are handled here and in the next patch.
Oh right; I completely forgot to mention that in the Changelog didn't I :-(
In this case, this driver is a broken piece of crap and doing fake idle with RR is just plain idiotic. Also note the WARNs in play_idle().
Also, rjw, what was the point of renmaing play_idle() to play_idle_precise() if there is only one anyway? The changelog talks about adding play_idle_precise() but the patch (rightfully) doesn't add another version but replaces the existing one. But why change the name?!
| |