linux-kernel - Re: [PATCH v8 1/1] genirq/devres: Add dev_err_probe() in devm_request_threaded_irq() and devm_request_any_context

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: <87qzxzjr3l.ffs@tglx>
Date: Tue, 29 Jul 2025 11:14:54 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Pan Chuang <panchuang@...o.com>, linux-kernel@...r.kernel.org
Cc: miquel.raynal@...tlin.com, Jonathan.Cameron@...wei.com,
 u.kleine-koenig@...gutronix.de, angeg.delregno@...labora.com,
 krzk@...nel.org, a.fatoum@...gutronix.de, frank.li@...o.com, Pan Chuang
 <panchuang@...o.com>
Subject: Re: [PATCH v8 1/1] genirq/devres: Add dev_err_probe() in
 devm_request_threaded_irq() and devm_request_any_context_irq()

On Tue, Jul 29 2025 at 16:14, Pan Chuang wrote:

> The devm_request_threaded_irq() and devm_request_any_context_irq() functions

devm_request_threaded_irq() and devm_request_any_context_irq() ....

The '()' notation already makes it clear that these are functions, so no
'The ... functions' is redundant.

> currently don't print any error when interrupt registration fails. This forces
> each driver to implement redundant error logging - over 2,000 lines of error
> messages exist across drivers. Additionally, when upper-layer functions
> propagate these errors without logging, critical debugging information is lost.
>
> Add automatic error logging to these functions via dev_err_probe(), printing
> device name, IRQ number, handler addresses, and error code on failure.

Again: %pS (or %ps) does NOT print the handler address. It prints the
symbol name. Feel free to ignore my review comments, but then accept
that I ignore your patches too.

> Signed-off-by: Yangtao Li <frank.li@...o.com>
> Signed-off-by: Pan Chuang <panchuang@...o.com>

This SOB chain is still incorrect. Again:

 https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

If anything is unclear, then please ask.

> +/**
> + * devm_request_any_context_irq - allocate an interrupt line for a managed device with error logging
> + * @dev:	Device to request interrupt for
> + * @irq:	Interrupt line to allocate
> + * @handler:	Function to be called when the IRQ occurs
> + * @irqflags:	Interrupt type flags
> + * @devname:	An ascii name for the claiming device, dev_name(dev) if NULL
> + * @dev_id:	A cookie passed back to the handler function
> + *
> + * Except for the extra @dev argument, this function takes the same arguments
> + * and performs the same function as request_any_context_irq().  IRQs requested
> + * with this function will be automatically freed on driver detach.
> + *
> + * If an IRQ allocated with this function needs to be freed separately,
> + * devm_free_irq() must be used.
> + *
> + * When the request fails, an error message is printed with contextual
> + * information (device name, interrupt number, handler functions and
> + * error code). Don't add extra error messages at the call sites.
> + *
> + * On failure, it returns a negative value. On success, it returns either
> + * IRQC_IS_HARDIRQ or IRQC_IS_NESTED.

As you touch this, can you please convert this to the proper

   Returns:

formatting?

Thanks,

        tglx