+CC stable
On 03/01/22 15:24, tip-bot2 for Valentin Schneider wrote:
> The following commit has been merged into the sched/core branch of tip:
>
> Commit-ID: fa2c3254d7cfff5f7a916ab928a562d1165f17bb
> Gitweb: https://git.kernel.org/tip/fa2c3254d7cfff5f7a916ab928a562d1165f17bb
> Author: Valentin Schneider <valentin.schneider(a)arm.com>
> AuthorDate: Thu, 20 Jan 2022 16:25:19
> Committer: Peter Zijlstra <peterz(a)infradead.org>
> CommitterDate: Tue, 01 Mar 2022 16:18:39 +01:00
>
> sched/tracing: Don't re-read p->state when emitting sched_switch event
>
> As of commit
>
> c6e7bd7afaeb ("sched/core: Optimize ttwu() spinning on p->on_cpu")
>
> the following sequence becomes possible:
>
> p->__state = TASK_INTERRUPTIBLE;
> __schedule()
> deactivate_task(p);
> ttwu()
> READ !p->on_rq
> p->__state=TASK_WAKING
> trace_sched_switch()
> __trace_sched_switch_state()
> task_state_index()
> return 0;
>
> TASK_WAKING isn't in TASK_REPORT, so the task appears as TASK_RUNNING in
> the trace event.
>
> Prevent this by pushing the value read from __schedule() down the trace
> event.
>
> Reported-by: Abhijeet Dharmapurikar <adharmap(a)quicinc.com>
> Signed-off-by: Valentin Schneider <valentin.schneider(a)arm.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz(a)infradead.org>
> Reviewed-by: Steven Rostedt (Google) <rostedt(a)goodmis.org>
> Link: https://lore.kernel.org/r/20220120162520.570782-2-valentin.schneider@arm.com
Any objection to picking this for stable? I'm interested in this one for some
Android users but prefer if it can be taken by stable rather than backport it
individually.
I think it makes sense to pick the next one in the series too.
Thanks!
--
Qais Yousef
+CC stable
On 01/20/22 16:25, Valentin Schneider wrote:
> TASK_RTLOCK_WAIT currently isn't part of TASK_REPORT, thus a task blocking
> on an rtlock will appear as having a task state == 0, IOW TASK_RUNNING.
>
> The actual state is saved in p->saved_state, but reading it after reading
> p->__state has a few issues:
> o that could still be TASK_RUNNING in the case of e.g. rt_spin_lock
> o ttwu_state_match() might have changed that to TASK_RUNNING
>
> As pointed out by Eric, adding TASK_RTLOCK_WAIT to TASK_REPORT implies
> exposing a new state to userspace tools which way not know what to do with
> them. The only information that needs to be conveyed here is that a task is
> waiting on an rt_mutex, which matches TASK_UNINTERRUPTIBLE - there's no
> need for a new state.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig(a)pengutronix.de>
> Signed-off-by: Valentin Schneider <valentin.schneider(a)arm.com>
Any objection for this to be picked up by stable? We care about Patch 1 only in
this series for stable, but it seems sensible to pick this one too, no strong
feeling if it is omitted though.
AFAICT it seems the problem dates back since commit:
1593baab910d ("sched/debug: Implement consistent task-state printing")
or even before. I think v4.14+ is good enough.
Thoughts?
Thanks!
--
Qais Yousef