lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <jeajjewfbg5qo736imozpghnpxln2pux74aegtqsi57qsbpug2@opndel6zc3m3>
Date: Wed, 2 Jul 2025 08:54:48 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Waqar Hameed <waqar.hameed@...s.com>, 
	Vignesh Raghavendra <vigneshr@...com>, Julien Panis <jpanis@...libre.com>, 
	William Breathitt Gray <wbg@...nel.org>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Peter Rosin <peda@...ntia.se>, 
	David Lechner <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, Cosmin Tanislav <cosmin.tanislav@...log.com>, 
	Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Matteo Martelli <matteomartelli3@...il.com>, 
	Heiko Stuebner <heiko@...ech.de>, Francesco Dolcini <francesco@...cini.it>, 
	João Paulo Gonçalves <jpaulo.silvagoncalves@...il.com>, Hugo Villeneuve <hvilleneuve@...onoff.com>, 
	Subhajit Ghosh <subhajit.ghosh@...aklogic.com>, Mudit Sharma <muditsharma.info@...il.com>, 
	Gerald Loacker <gerald.loacker@...fvision.net>, Song Qiang <songqiang1304521@...il.com>, 
	Crt Mori <cmo@...exis.com>, Dmitry Torokhov <dmitry.torokhov@...il.com>, 
	Ulf Hansson <ulf.hansson@...aro.org>, Karol Gugala <kgugala@...micro.com>, 
	Mateusz Holenko <mholenko@...micro.com>, Gabriel Somlo <gsomlo@...il.com>, Joel Stanley <joel@....id.au>, 
	Claudiu Manoil <claudiu.manoil@....com>, Vladimir Oltean <vladimir.oltean@....com>, 
	Wei Fang <wei.fang@....com>, Clark Wang <xiaoning.wang@....com>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Vinod Koul <vkoul@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Alim Akhtar <alim.akhtar@...sung.com>, Sebastian Reichel <sre@...nel.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Kevin Hilman <khilman@...libre.com>, 
	Jerome Brunet <jbrunet@...libre.com>, Martin Blumenstingl <martin.blumenstingl@...glemail.com>, 
	Han Xu <han.xu@....com>, Haibo Chen <haibo.chen@....com>, 
	Yogesh Gaur <yogeshgaur.83@...il.com>, Mark Brown <broonie@...nel.org>, 
	Avri Altman <avri.altman@....com>, Bart Van Assche <bvanassche@....org>, 
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>, "Martin K. Petersen" <martin.petersen@...cle.com>, 
	Souradeep Chowdhury <quic_schowdhu@...cinc.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Liam Girdwood <lgirdwood@...il.com>, Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>, 
	Bard Liao <yung-chuan.liao@...ux.intel.com>, Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>, 
	Daniel Baluta <daniel.baluta@....com>, Kai Vehmanen <kai.vehmanen@...ux.intel.com>, 
	Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>, Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, kernel@...s.com, 
	linux-iio@...r.kernel.org, linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-gpio@...r.kernel.org, linux-i2c@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-input@...r.kernel.org, 
	linux-mmc@...r.kernel.org, imx@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-phy@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, linux-pm@...r.kernel.org, 
	linux-pwm@...r.kernel.org, linux-amlogic@...ts.infradead.org, linux-spi@...r.kernel.org, 
	linux-scsi@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org, 
	sound-open-firmware@...a-project.org, linux-sound@...r.kernel.org
Subject: Re: [PATCH] Remove error prints for devm_add_action_or_reset()

Hello Jonathan,

On Tue, Jul 01, 2025 at 06:55:19PM +0100, Jonathan Cameron wrote:
> On Tue, 1 Jul 2025 19:44:17 +0200
> Uwe Kleine-König <ukleinek@...nel.org> wrote:
> 
> > On Tue, Jul 01, 2025 at 05:03:33PM +0200, Waqar Hameed wrote:
> > >  drivers/pwm/pwm-meson.c                          | 3 +--  
> > 
> > Looking at this driver I tried the following:
> 
> I'm not sure what we actually want here.
> 
> My thought when suggesting removing instances of this
> particular combination wasn't saving on code size, but rather just
> general removal of pointless code that was getting cut and
> paste into new drivers and wasting a tiny bit of review bandwidth.
> I'd consider it bad practice to have patterns like
> 
> void *something = kmalloc();
> if  (!something)
> 	return dev_err_probe(dev, -ENOMEM, ..);
> 
> and my assumption was people would take a similar view with
> devm_add_action_or_reset().
>
> It is a bit nuanced to have some cases where we think prints
> are reasonable and others where they aren't so I get your
> point about consistency.

The problem I see is that there are two classes of functions: a) Those
that require an error message and b) those that don't. Class b) consists
of the functions that can only return success or -ENOMEM and the
functions that emit an error message themselves. (And another problem I
see is that for the latter the error message is usually non-optimal
because the function doesn't know the all details of the request. See my
reply to Andy for more details about that rant.)

IMHO what takes away the review bandwidth is that the reviewer has to
check which class the failing function is part of. If this effort
results in more driver authors not adding an error message after
devm_add_action_or_reset() that's nice, but in two months I have
forgotten the details of this discussion and I have to recheck if
devm_add_action_or_reset() is part of a) or b) and so the burden is
still on me.

So to give my answer on your question "What do we actually want here?":
Please let us get rid of the need to care for a) or b).

> The code size reduction is nice so I'd not be against it as an extra
> if the reduction across a kernel builds is significant and enough
> people want to keep these non printing prints.

To complete implementing my wish all API functions would need to stop to
emit an error message. Unfortunately that isn't without downsides
because the result is that there are more error strings and so the
kernel size is increased. So you have to weight if you prefer individual
error messages and easier review/maintenance at the cost of a bigger
binary size and more dev_err_probe() calls in drivers eating vertical
space in your editor.

I know on which side I am, but I bet we won't find agreement about that
in the kernel community ...

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ