linux-kernel - Re: [PATCH v9 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: <8b5162ba-2419-4552-9623-d73772d398b1@vivo.com>
Date: Thu, 31 Jul 2025 10:44:49 +0800
From: PanChuang <panchuang@...o.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>, tglx@...utronix.de
Cc: kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org,
 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
Subject: Re: [PATCH v9 1/1] genirq/devres: Add dev_err_probe() in
 devm_request_threaded_irq() and devm_request_any_context_irq()

Hi, Christophe

在 2025/7/31 0:48, Christophe JAILLET 写道:
> Le 30/07/2025 à 08:25, Pan Chuang a écrit :
>> + * Return: IRQC_IS_HARDIRQ or IRQC_IS_NESTED on success, or a 
>> negative error
>> + * number.
>> + */
>> +int devm_request_any_context_irq(struct device *dev, unsigned int irq,
>> +                 irq_handler_t handler, unsigned long irqflags,
>> +                 const char *devname, void *dev_id)
>> +{
>> +    int rc = __devm_request_any_context_irq(dev, irq, handler, 
>> irqflags,
>> +                        devname, dev_id);
>> +    if (rc < 0) {
>> +        return dev_err_probe(dev, rc, "request_irq(%u) %ps %s\n",
>> +                     irq, handler, devname ? : "");
>> +    }
>
> Extra { } should be removed.
>
> From my PoV, it would look more logical to have the same logic in 
> devm_request_threaded_irq() and in devm_request_any_context_irq().
>
> Why "if (!rc) SUCCESS" in one case, and "if (rc < 0) FAILURE" in the 
> other case?
>
> Personally, I would change in devm_request_threaded_irq() above to have
>     if (rc)
>         return dev_err_probe();
>     return 0;
>
Thanks for your suggestion.

The return value of __devm_request_any_context_irq on success is either 
IRQC_IS_HARDIRQ or IRQC_IS_NESTED (>= 0).

Therefore, we cannot directly use "if (rc)" to check for errors.

>> +
>> +    return rc;
>> +}
>>   EXPORT_SYMBOL(devm_request_any_context_irq);
>
> On version 5 of the patch, there was a comment related to using 
> EXPORT_SYMBOL_GPL(), does it still make sense?
> (no strong opinion from me, just noted that and wondered if done on 
> purpose)
>
Since it is an existing export, I will keep it as-is.

Thanks,

     Pan Chuang