From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E406C43217 for ; Wed, 20 Apr 2022 17:36:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1381236AbiDTRjY (ORCPT ); Wed, 20 Apr 2022 13:39:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235570AbiDTRjW (ORCPT ); Wed, 20 Apr 2022 13:39:22 -0400 Received: from mail-yw1-f173.google.com (mail-yw1-f173.google.com [209.85.128.173]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B02291EC74; Wed, 20 Apr 2022 10:36:35 -0700 (PDT) Received: by mail-yw1-f173.google.com with SMTP id 00721157ae682-2ef4a241cc5so26269767b3.2; Wed, 20 Apr 2022 10:36:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=aSRYCi9Gu592mc1yy2JGCfSHyy3Om8/ZMl9fXbJ4VAk=; b=ryiG9d/g0nAi1suycm097/0o0XNUzQhQjQg2vmNWQnRIOguh9XoKKkRSiJfbsSG5tc vQCNo0+01j4eJ/mQfLJcTDSnBGx0HESqEoorumqIjkHocbCnSVc5AO2hUQaKCxpHJfD2 s2KMfwrawC2ufC8pAbxBn3W72mUZ9NI9d2oRXgGfz3PXSI7erLu0Q5MD0XOYyaXX78Sx fg4hKI+bEsO3JvRcsXv0Sn1ykDteglvIOY2N+CC+1I/iYweTYb6H0HU6WcExdZlSZYJ7 l1WkMOsOj3TKNwSoKoqzcdVgM+5SFmr28Y80vxOn2DL/VKlgEYe2TJ7csyQhLyEt5Bjs x9DA== X-Gm-Message-State: AOAM5303P3RyWlc6t0fECgadJ9ipaqrwLxeq39r1vXCOuBSgSkxTb02D kFvVqgxkjJt2E26sxkyIsRmjrANlGzBEx4znGxY= X-Google-Smtp-Source: ABdhPJzNeFf91aJ2AcO4HxwsyceUmkMGZe3BT/sKo5s3TT3nCPguaeHFtXhd48Duxnk7AG97fF2bO/GqS3ZkGsIQMYE= X-Received: by 2002:a81:260a:0:b0:2f4:ca82:a42f with SMTP id m10-20020a81260a000000b002f4ca82a42fmr1985644ywm.149.1650476194968; Wed, 20 Apr 2022 10:36:34 -0700 (PDT) MIME-Version: 1.0 References: <20220411233832.391817-1-dmitry.osipenko@collabora.com> <20220411233832.391817-4-dmitry.osipenko@collabora.com> In-Reply-To: From: "Rafael J. Wysocki" Date: Wed, 20 Apr 2022 19:36:23 +0200 Message-ID: Subject: Re: [PATCH v7 03/20] reboot: Print error message if restart handler has duplicated priority To: Dmitry Osipenko Cc: "Rafael J. Wysocki" , Thierry Reding , Jonathan Hunter , Russell King , Catalin Marinas , Will Deacon , Guo Ren , Geert Uytterhoeven , Greg Ungerer , Joshua Thompson , Thomas Bogendoerfer , Sebastian Reichel , Linus Walleij , Philipp Zabel , Greentime Hu , Vincent Chen , "James E.J. Bottomley" , Helge Deller , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Yoshinori Sato , Rich Felker , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "the arch/x86 maintainers" , "H. Peter Anvin" , Boris Ostrovsky , Juergen Gross , Stefano Stabellini , Len Brown , Santosh Shilimkar , Krzysztof Kozlowski , Liam Girdwood , Mark Brown , Pavel Machek , Lee Jones , Andrew Morton , Guenter Roeck , Daniel Lezcano , Andy Shevchenko , Ulf Hansson , =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= , Linux Kernel Mailing List , linux-csky@vger.kernel.org, linux-ia64@vger.kernel.org, linux-m68k@lists.linux-m68k.org, "open list:BROADCOM NVRAM DRIVER" , linux-parisc@vger.kernel.org, linux-riscv@lists.infradead.org, Linux-sh list , xen-devel@lists.xenproject.org, ACPI Devel Maling List , Linux PM , linux-tegra Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 18, 2022 at 3:29 AM Dmitry Osipenko wrote: > > On 4/14/22 14:19, Rafael J. Wysocki wrote: > > On Thu, Apr 14, 2022 at 12:24 AM Dmitry Osipenko > > wrote: > >> > >> On 4/13/22 21:48, Rafael J. Wysocki wrote: > >>> On Tue, Apr 12, 2022 at 1:39 AM Dmitry Osipenko > >>> wrote: > >>>> > >>>> Add sanity check which ensures that there are no two restart handlers > >>>> registered using the same priority. This requirement will become mandatory > >>>> once all drivers will be converted to the new API and such errors will be > >>>> fixed. > >>>> > >>>> Signed-off-by: Dmitry Osipenko > >>> > >>> The first two patches in the series are fine with me and there's only > >>> one minor nit regarding this one (below). > >>> > >>>> --- > >>>> kernel/reboot.c | 15 +++++++++++++++ > >>>> 1 file changed, 15 insertions(+) > >>>> > >>>> diff --git a/kernel/reboot.c b/kernel/reboot.c > >>>> index ed4e6dfb7d44..acdae4e95061 100644 > >>>> --- a/kernel/reboot.c > >>>> +++ b/kernel/reboot.c > >>>> @@ -182,6 +182,21 @@ static ATOMIC_NOTIFIER_HEAD(restart_handler_list); > >>>> */ > >>>> int register_restart_handler(struct notifier_block *nb) > >>>> { > >>>> + int ret; > >>>> + > >>>> + ret = atomic_notifier_chain_register_unique_prio(&restart_handler_list, nb); > >>>> + if (ret != -EBUSY) > >>>> + return ret; > >>>> + > >>>> + /* > >>>> + * Handler must have unique priority. Otherwise call order is > >>>> + * determined by registration order, which is unreliable. > >>>> + * > >>>> + * This requirement will become mandatory once all drivers > >>>> + * will be converted to use new sys-off API. > >>>> + */ > >>>> + pr_err("failed to register restart handler using unique priority\n"); > >>> > >>> I would use pr_info() here, because this is not a substantial error AFAICS. > >> > >> It's indeed not a substantial error so far, but it will become > >> substantial later on once only unique priorities will be allowed. The > >> pr_warn() could be a good compromise here, pr_info() is too mild, IMO. > > > > Well, I'm still unconvinced about requiring all of the users of this > > interface to use unique priorities. > > > > Arguably, there are some of them who don't really care about the > > ordering, so could there be an option for them to specify the lack of > > care by, say, passing 0 as the priority that would be regarded as a > > special case? > > > > IOW, if you pass 0, you'll be run along the others who've also passed > > 0, but if you pass anything different from 0, it must be unique. What > > do you think? > > There are indeed cases where ordering is unimportant. Like a case of > PMIC and watchdog restart handlers for example, both handlers will > produce equal effect from a user's perspective. Perhaps indeed it's more > practical to have at least one shared level. > > In this patchset the level 0 is specified as an alias to the default > level 128. If one user registers handler using unique level 128 and the > other user uses non-unique level 0, then we have ambiguity. > > One potential option is to make the whole default level 128 non-unique. > This will allow users to not care about the uniqueness by default like > they always did it previously, but it will hide potential problems for > users who actually need unique level and don't know about it yet due to > a lucky registration ordering that they have today. Are you okay with > this option? Yes, I am.