[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200501193502.7c9b2b10@oasis.local.home>
Date: Fri, 1 May 2020 19:35:02 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, mhiramat@...nel.org,
bristot@...hat.com, jbaron@...mai.com,
torvalds@...ux-foundation.org, tglx@...utronix.de,
mingo@...nel.org, namit@...are.com, hpa@...or.com, luto@...nel.org,
ard.biesheuvel@...aro.org, jpoimboe@...hat.com,
pbonzini@...hat.com, mathieu.desnoyers@...icios.com,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH v4 01/18] notifier: Fix broken error handling pattern
On Fri, 01 May 2020 22:28:50 +0200
Peter Zijlstra <peterz@...radead.org> wrote:
> The current notifiers have the following error handling pattern all
> over the place:
>
> int err, nr;
>
> err = __foo_notifier_call_chain(&chain, val_up, v, -1, &nr);
> if (err & NOTIFIER_STOP_MASK)
> __foo_notifier_call_chain(&chain, val_down, v, nr-1, NULL)
>
> And aside from the endless repetition thereof, it is broken. Consider
> blocking notifiers; both calls take and drop the rwsem, this means
> that the notifier list can change in between the two calls, making @nr
> meaningless.
>
> Fix this by replacing all the __foo_notifier_call_chain() functions
> with foo_notifier_call_chain_robust() that embeds the above pattern,
> but ensures it is inside a single lock region.
>
> Note: I switched atomic_notifier_call_chain_robust() to use
> the spinlock, since RCU cannot provide the guarantee
> required for the recovery.
>
> Note: software_resume() error handling was broken afaict.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> include/linux/notifier.h | 15 +--
> kernel/cpu_pm.c | 48 ++++--------
> kernel/notifier.c | 144 ++++++++++++++++++++++---------------
> kernel/power/hibernate.c | 26 +++---
> kernel/power/main.c | 8 +-
> kernel/power/power.h | 3
> kernel/power/suspend.c | 14 +--
> kernel/power/user.c | 14 +--
> tools/power/pm-graph/sleepgraph.py | 2
> 9 files changed, 141 insertions(+), 133 deletions(-)
>
This looks like something that can go in outside this series.
-- Steve
Powered by blists - more mailing lists