On Wed, Nov 08, 2023 at 02:26:37AM -0800, Ankur Arora wrote:
>
> Peter Zijlstra <peterz@xxxxxxxxxxxxx> writes:
>
> > On Tue, Nov 07, 2023 at 01:57:27PM -0800, Ankur Arora wrote:
> >
> >> --- a/kernel/sched/core.c
> >> +++ b/kernel/sched/core.c
> >> @@ -1027,13 +1027,13 @@ void wake_up_q(struct wake_q_head *head)
> >> }
> >>
> >> /*
> >> - * resched_curr - mark rq's current task 'to be rescheduled now'.
> >> + * __resched_curr - mark rq's current task 'to be rescheduled'.
> >> *
> >> - * On UP this means the setting of the need_resched flag, on SMP it
> >> - * might also involve a cross-CPU call to trigger the scheduler on
> >> - * the target CPU.
> >> + * On UP this means the setting of the need_resched flag, on SMP, for
> >> + * eager resched it might also involve a cross-CPU call to trigger
> >> + * the scheduler on the target CPU.
> >> */
> >> -void resched_curr(struct rq *rq)
> >> +void __resched_curr(struct rq *rq, resched_t rs)
> >> {
> >> struct task_struct *curr = rq->curr;
> >> int cpu;
> >> @@ -1046,17 +1046,77 @@ void resched_curr(struct rq *rq)
> >> cpu = cpu_of(rq);
> >>
> >> if (cpu == smp_processor_id()) {
> >> - set_tsk_need_resched(curr, RESCHED_eager);
> >> - set_preempt_need_resched();
> >> + set_tsk_need_resched(curr, rs);
> >> + if (rs == RESCHED_eager)
> >> + set_preempt_need_resched();
> >> return;
> >> }
> >>
> >> - if (set_nr_and_not_polling(curr, RESCHED_eager))
> >> - smp_send_reschedule(cpu);
> >> - else
> >> + if (set_nr_and_not_polling(curr, rs)) {
> >> + if (rs == RESCHED_eager)
> >> + smp_send_reschedule(cpu);
> >
> > I think you just broke things.
> >
> > Not all idle threads have POLLING support, in which case you need that
> > IPI to wake them up, even if it's LAZY.
>
> Yes, I was concerned about that too. But doesn't this check against the
> idle_sched_class in resched_curr() cover that?
I that's what that was. Hmm, maybe.
I mean, we have idle-injection too, those don't as FIFO, but as such,
they can only get preempted from RT/DL, and those will already force
preempt anyway.
The way you've split and structured the code makes it very hard to
follow. Something like:
if (set_nr_and_not_polling(curr, rs) &&
(rs == RESCHED_force || is_idle_task(curr)))
smp_send_reschedule();
is *far* clearer, no?
