Messages in this thread | | | Date | Tue, 8 Nov 2022 10:11:49 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] sched/core: Minor optimize ttwu_runnable() |
| |
On Mon, Nov 07, 2022 at 03:54:38PM +0000, Valentin Schneider wrote:
> So that's the part for the p->sched_class->task_woken() callback, which > only affects RT and DL (and only does something when !p->on_cpu). I *think* > it's fine to remove it from ttwu_runnable() as any push/pull should have > happened when other tasks were enqueued on the same CPU - with that said, > it wouldn't hurt to double check this :-) > > > As for the check_preempt_curr(), since per the above p can be preempted, > you could have scenarios right now with CFS tasks where > ttwu_runnable()->check_preempt_curr() causes NEED_RESCHED to be set. > > e.g. p0 does > > set_current_state(TASK_UNINTERRUPTIBLE) > > but then gets interrupted by the tick, a p1 gets selected to run instead > because of check_preempt_tick(), and then runs long enough to have > check_preempt_curr() decide to let p0 preempt p1. > > That does require specific timing (lower tick frequency should make this > more likely) and probably task niceness distribution too, but isn't > impossible. > > Maybe try reading p->on_cpu, and only do the quick task state update if > it's still the current task, otherwise do the preemption checks?
I'm confused...
So all relevant parties take rq->lock:
- __schedule() - scheduler_tick() - ttwu_runnable()
So if ttwu_runnable() sees on_rq and switches state back to RUNNING then neither check_preempt_curr() nor task_woken() make any sense.
Specifically:
- you can't very well preempt yourself (which is what check_preempt_curr() is trying to determine -- if the woken task should preempt the running task, they're both the same in this case);
- the task did not actually wake up, because it was still on the runqueue to begin with. This path prevents a sleep, rather than issues a wakeup.
So yes, I think the patch as proposed is ok.
| |