linux-kernel - Re: [PATCH v2 01/15] genirq/devres: Add error information printing for devm_request_threaded

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Jun 2023 12:28:14 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Yangtao Li <frank.li@...o.com>, miquel.raynal@...tlin.com,
        rafael@...nel.org, daniel.lezcano@...aro.org, amitk@...nel.org,
        rui.zhang@...el.com, mmayer@...adcom.com,
        bcm-kernel-feedback-list@...adcom.com,
        florian.fainelli@...adcom.com, shawnguo@...nel.org,
        s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
        linux-imx@....com, agross@...nel.org, andersson@...nel.org,
        konrad.dybcio@...aro.org, thara.gopinath@...il.com,
        heiko@...ech.de, mcoquelin.stm32@...il.com,
        alexandre.torgue@...s.st.com, thierry.reding@...il.com,
        jonathanh@...dia.com, tglx@...utronix.de, matthias.bgg@...il.com,
        angelogioacchino.delregno@...labora.com,
        srinivas.pandruvada@...ux.intel.com,
        DLG-Adam.Ward.opensource@...renesas.com, shangxiaojing@...wei.com,
        bchihi@...libre.com, wenst@...omium.org,
        u.kleine-koenig@...gutronix.de, hayashi.kunihiko@...ionext.com,
        niklas.soderlund+renesas@...natech.se, chi.minghao@....com.cn,
        johan+linaro@...nel.org, jernej.skrabec@...il.com
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-tegra@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 01/15] genirq/devres: Add error information printing
 for devm_request_threaded_irq()

On 27/06/2023 12:12, Yangtao Li wrote:
> Ensure that all error handling branches print error information. In this
> way, when this function fails, the upper-layer functions can directly
> return an error code without missing debugging information. Otherwise,
> the error message will be printed redundantly or missing.
> 
> There are more than 700 calls to the devm_request_threaded_irq method.
> Most drivers only request one interrupt resource, and these error
> messages are basically the same. If error messages are printed
> everywhere, more than 1000 lines of code can be saved by removing the
> msg in the driver.
> 
> Signed-off-by: Yangtao Li <frank.li@...o.com>
> ---
>  kernel/irq/devres.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
> index f6e5515ee077..fcb946ffb7ec 100644
> --- a/kernel/irq/devres.c
> +++ b/kernel/irq/devres.c
> @@ -58,8 +58,10 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
>  
>  	dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
>  			  GFP_KERNEL);
> -	if (!dr)
> +	if (!dr) {
> +		dev_err(dev, "Failed to allocate device resource data\n");

I don't understand why did you send v2:
1. Without responding to my comments - either by implementing them or
continuing the discussion
2. Without changelog explaining what happened here

My comments for v1 stand. Please do not ignore them, respond. If sending
new version, then usually one per day is max and of course provide
changelog.

>  		return -ENOMEM;
> +	}
>  
>  	if (!devname)
>  		devname = dev_name(dev);
> @@ -67,6 +69,7 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
>  	rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname,
>  				  dev_id);
>  	if (rc) {
> +		dev_err_probe(dev, rc, "Failed to request threaded irq%d: %d\n", irq, rc);

Why printing rc twice? Did you test this patch? Does not look like.

Best regards,
Krzysztof