Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 1 May 2020 19:35:02 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org, mhiramat@kernel.org,
        bristot@redhat.com, jbaron@akamai.com,
        torvalds@linux-foundation.org, tglx@linutronix.de,
        mingo@kernel.org, namit@vmware.com, hpa@zytor.com, luto@kernel.org,
        ard.biesheuvel@linaro.org, jpoimboe@redhat.com,
        pbonzini@redhat.com, mathieu.desnoyers@efficios.com,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: [PATCH v4 01/18] notifier: Fix broken error handling pattern
Message-ID: <20200501193502.7c9b2b10@oasis.local.home>
In-Reply-To: <20200501202943.837409514@infradead.org>
References: <20200501202849.647891881@infradead.org>
        <20200501202943.837409514@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 01 May 2020 22:28:50 +0200
Peter Zijlstra <peterz@infradead.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@infradead.org>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.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