netdev - Re: [PATCH] Remove error prints for devm_add_action_or

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: <ylr7cuxldwb24ccenen4khtyddzq3owgzzfblbohkdxb7p7eeo@qpuddn6wrz3x>
Date: Tue, 1 Jul 2025 19:44:17 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Waqar Hameed <waqar.hameed@...s.com>
Cc: 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>, Jonathan Cameron <jic23@...nel.org>, 
	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,

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:

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

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