LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Borislav Petkov <bp@alien8.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnd Bergmann <arnd@arndb.de>,
	Ayush Sawal <ayush.sawal@chelsio.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rohit Maheshwari <rohitm@chelsio.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Vinay Kumar Yadav <vinay.yadav@chelsio.com>,
	ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	intel-gvt-dev@lists.freedesktop.org,
	alpha <linux-alpha@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	linux-edac@vger.kernel.org,
	Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
	linux-hyperv@vger.kernel.org, linux-iio@vger.kernel.org,
	linux-leds <linux-leds@vger.kernel.org>,
	"open list:BROADCOM NVRAM DRIVER" <linux-mips@vger.kernel.org>,
	Parisc List <linux-parisc@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM" 
	<linux-remoteproc@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	scsi <linux-scsi@vger.kernel.org>,
	Linux-sh list <linux-sh@vger.kernel.org>,
	linux-staging@lists.linux.dev,
	linux-tegra <linux-tegra@vger.kernel.org>,
	linux-um <linux-um@lists.infradead.org>,
	USB list <linux-usb@vger.kernel.org>,
	"open list:TENSILICA XTENSA PORT (xtensa)" 
	<linux-xtensa@linux-xtensa.org>, netdev <netdev@vger.kernel.org>,
	openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org,
	sparclinux <sparclinux@vger.kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered
Date: Mon, 8 Nov 2021 16:25:47 +0100	[thread overview]
Message-ID: <CAMuHMdXiBEQyEXJagSfpH44hxVA2t0sDH7B7YubLGHrb2MJLLA@mail.gmail.com> (raw)
In-Reply-To: <YYkyUEqcsOwQMb1S@zn.tnic>

Hi Borislav,

On Mon, Nov 8, 2021 at 3:21 PM Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Nov 08, 2021 at 03:07:03PM +0100, Geert Uytterhoeven wrote:
> > I think the addition of __must_check is overkill, leading to the
> > addition of useless error checks and message printing.
>
> See the WARN in notifier_chain_register() - it will already do "message
> printing".

I mean the addition of useless error checks and message printing _to
the callers_.

> > Many callers call this where it cannot fail, and where nothing can
> > be done in the very unlikely event that the call would ever start to
> > fail.
>
> This is an attempt to remove this WARN() hack in
> notifier_chain_register() and have the function return a proper error
> value instead of this "Currently always returns zero." which is bad
> design.
>
> Some of the registration functions around the tree check that retval and
> some don't. So if "it cannot fail" those registration either should not
> return a value or callers should check that return value - what we have
> now doesn't make a whole lot of sense.

With __must_check callers are required to check, even if they know
it cannot fail.

> Oh, and then fixing this should avoid stuff like:
>
> +       if (notifier_registered == false) {
> +               mce_register_decode_chain(&amdgpu_bad_page_nb);
> +               notifier_registered = true;
> +       }
>
> from propagating in the code.

That's unrelated to the addition of __must_check.

I'm not against returning proper errors codes.  I'm against forcing
callers to check things that cannot fail and to add individual error
printing to each and every caller.

Note that in other areas, we are moving in the other
direction, to a centralized printing of error messages,
cfr. e.g. commit 7723f4c5ecdb8d83 ("driver core: platform: Add an
error message to platform_get_irq*()").

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply	other threads:[~2021-11-08 15:33 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-08 10:11 [PATCH v0 00/42] notifiers: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 01/42] x86: Check notifier registration return value Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 02/42] xen/x86: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 03/42] impi: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 04/42] clk: renesas: " Borislav Petkov
2021-11-08 14:12   ` Geert Uytterhoeven
2021-11-08 10:11 ` [PATCH v0 05/42] dca: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 06/42] firmware: " Borislav Petkov
2021-11-12 18:43   ` Cristian Marussi
2021-11-08 10:11 ` [PATCH v0 07/42] drm/i915: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 08/42] Drivers: hv: vmbus: " Borislav Petkov
2021-11-08 11:16   ` Wei Liu
2021-11-08 11:39     ` Borislav Petkov
2021-11-08 11:45       ` Wei Liu
2021-11-08 12:08         ` Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 09/42] iio: proximity: cros_ec: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 10/42] leds: trigger: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 11/42] misc: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 12/42] ethernet: chelsio: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 13/42] power: reset: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 14/42] remoteproc: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 15/42] scsi: target: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 16/42] USB: " Borislav Petkov
2021-11-08 14:05   ` Alan Stern
2021-11-08 14:09     ` Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 17/42] drivers: video: " Borislav Petkov
2021-11-08 11:16   ` Wei Liu
2021-11-08 10:11 ` [PATCH v0 18/42] drivers/xen: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 19/42] kernel/hung_task: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 20/42] rcu: " Borislav Petkov
2021-11-08 16:53   ` Paul E. McKenney
2021-11-08 10:11 ` [PATCH v0 21/42] tracing: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 22/42] net: fib_notifier: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 23/42] ASoC: soc-jack: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 24/42] staging: olpc_dcon: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 25/42] arch/um: " Borislav Petkov
2021-11-08 10:23   ` Johannes Berg
2021-11-08 10:48     ` Borislav Petkov
2021-11-08 10:56       ` Johannes Berg
2021-11-08 10:11 ` [PATCH v0 26/42] alpha: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 27/42] bus: brcmstb_gisb: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 28/42] soc: bcm: brcmstb: pm: pm-arm: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 29/42] arm64: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 30/42] soc/tegra: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 31/42] parisc: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 32/42] macintosh/adb: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 33/42] mips: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 34/42] powerpc: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 35/42] sh: " Borislav Petkov
2021-11-08 13:31   ` Geert Uytterhoeven
2021-11-08 13:49     ` Borislav Petkov
2021-11-08 14:03       ` Geert Uytterhoeven
2021-11-08 14:48   ` Sergey Shtylyov
2021-11-08 10:11 ` [PATCH v0 36/42] s390: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 37/42] sparc: " Borislav Petkov
2021-11-08 19:59   ` David Miller
2021-11-08 10:11 ` [PATCH v0 38/42] xtensa: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 39/42] crypto: ccree - check " Borislav Petkov
2021-11-12 18:32   ` Cristian Marussi
2021-11-12 18:48     ` Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 40/42] EDAC/altera: Check " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 41/42] power: supply: ab8500: " Borislav Petkov
2021-11-08 10:11 ` [PATCH v0 42/42] notifier: Return an error when callback is already registered Borislav Petkov
2021-11-08 14:07   ` Geert Uytterhoeven
2021-11-08 14:21     ` Borislav Petkov
2021-11-08 15:25       ` Geert Uytterhoeven [this message]
2021-11-08 15:58         ` Borislav Petkov
2021-11-08 16:12           ` Geert Uytterhoeven
2021-11-08 16:21             ` Borislav Petkov
2021-11-08 20:59               ` Alan Stern
2021-11-08 21:18                 ` Borislav Petkov
2021-11-08 10:19 ` [PATCH v0 00/42] notifiers: " Borislav Petkov
2021-11-08 14:17   ` Alan Stern
2021-11-08 14:24     ` Borislav Petkov
2021-11-08 14:35       ` Borislav Petkov
2021-11-08 16:23         ` Steven Rostedt
2021-11-08 16:29           ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMuHMdXiBEQyEXJagSfpH44hxVA2t0sDH7B7YubLGHrb2MJLLA@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnd@arndb.de \
    --cc=ayush.sawal@chelsio.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bp@alien8.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-gvt-dev@lists.freedesktop.org \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=rcu@vger.kernel.org \
    --cc=rohitm@chelsio.com \
    --cc=rostedt@goodmis.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=vinay.yadav@chelsio.com \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).