| 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
| ||
|
Message-ID: <20250701185519.1410e831@jic23-huawei>
Date: Tue, 1 Jul 2025 18:55:19 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Uwe Kleine-König <ukleinek@...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()
On Tue, 1 Jul 2025 19:44:17 +0200
Uwe Kleine-König <ukleinek@...nel.org> wrote:
> Hello,
>
> 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:
>
Hi Uqe,
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 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.
Jonathan
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3809baed42f3..58a2ab74f14c 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -5062,7 +5062,7 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
> *
> * Returns @err.
> */
> -int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
> +int _dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
> {
> va_list vargs;
>
> @@ -5075,7 +5075,7 @@ int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
>
> return err;
> }
> -EXPORT_SYMBOL_GPL(dev_err_probe);
> +EXPORT_SYMBOL_GPL(_dev_err_probe);
>
> /**
> * dev_warn_probe - probe error check and log helper
> diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
> index eb2094e43050..23ef250727f1 100644
> --- a/include/linux/dev_printk.h
> +++ b/include/linux/dev_printk.h
> @@ -275,7 +275,13 @@ do { \
> WARN_ONCE(condition, "%s %s: " format, \
> dev_driver_string(dev), dev_name(dev), ## arg)
>
> -__printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> +__printf(3, 4) int _dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> +
> +#define dev_err_probe(dev, err, ...) ( \
> + (__builtin_constant_p(err) && err == -ENOMEM) ? err : _dev_err_probe(dev, err, __VA_ARGS__) \
> +)
> +
> +
> __printf(3, 4) int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...);
>
> /* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */
> diff --git a/include/linux/device/devres.h b/include/linux/device/devres.h
> index ae696d10faff..abfa5152b5a7 100644
> --- a/include/linux/device/devres.h
> +++ b/include/linux/device/devres.h
> @@ -157,8 +157,11 @@ static inline int __devm_add_action_or_reset(struct device *dev, void (*action)(
> int ret;
>
> ret = __devm_add_action(dev, action, data, name);
> - if (ret)
> + if (ret) {
> + if (ret != -ENOMEM)
> + __builtin_unreachable();
> action(data);
> + }
>
> return ret;
> }
>
> With that
>
> ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
> meson->channels[i].clk);
> if (ret)
> return dev_err_probe(dev, ret,
> "Failed to add clk_put action\n");
>
> from drivers/pwm/pwm-meson.c is optimized to
>
> ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
> meson->channels[i].clk);
> if (ret)
> return ret;
>
> .
>
> I would prefer this approach, because a) there is no need to drop all
> dev_err_probe()s after devm_add_action_or_reset() and b) the
> dev_err_probe()s could stay for consistency in the error paths of a
> driver.
>
> Best regards
> Uwe
Powered by blists - more mailing lists