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] [day] [month] [year] [list]
Message-ID: <87sebrbenj.ffs@tglx>
Date: Tue, 27 Jan 2026 10:14:56 +0100
From: Thomas Gleixner <tglx@...nel.org>
To: Marek Vasut <marek.vasut+renesas@...lbox.org>, linux-input@...r.kernel.org
Cc: Marek Vasut <marek.vasut+renesas@...lbox.org>, "Peter Zijlstra (Intel)"
 <peterz@...radead.org>, Cheng-Yang Chou <yphbchou0911@...il.com>, Dmitry
 Torokhov <dmitry.torokhov@...il.com>, Frank Li <Frank.Li@....com>, Geert
 Uytterhoeven <geert+renesas@...der.be>, Jinjie Ruan
 <ruanjinjie@...wei.com>, Krzysztof Kozlowski
 <krzysztof.kozlowski@....qualcomm.com>, Marc Zyngier <maz@...nel.org>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH 1/2] linux/interrupt.h: allow "guard" notation to
 disable and reenable IRQ with valid IRQ check

On Thu, Jan 22 2026 at 00:23, Marek Vasut wrote:

$Subject: linux/interrupt.h is not a proper prefix. Just use 'genirq:'

Also what means 'allow ....'? Subject lines should provide a concise
summary of the change, i.e. something like:

        Provide conditional disable_irq guard variant

> Introduce disable_valid_irq scoped guard. This is an extension
> of disable_irq scoped guard, which disables and enables IRQs
> around a scope. The disable_valid_irq scoped guard does almost
> the same, except it handles the case where IRQ is not valid,
> in which case it does not do anything. This is meant to be used
> by for example touch controller drivers, which can do both IRQ
> driven and polling mode of operation, and this makes their code
> slighly simpler.

How many of those have this pattern? From a cursory look I found not
even one as your example is neither applicable to upstream nor to next.

> +static inline void disable_valid_irq(unsigned int irq)
> +{
> +	if (irq > 0)
> +		disable_irq(irq);
> +}
> +
> +static inline void enable_valid_irq(unsigned int irq)
> +{
> +	if (irq > 0)
> +		enable_irq(irq);
> +}
> +
> +DEFINE_LOCK_GUARD_1(disable_valid_irq, int,

'int' because it matches the function argument type of the inlines,
right?

> +		    disable_valid_irq(*_T->lock), enable_valid_irq(*_T->lock))

disable_valid_irq is a pretty non-intuitive name if you look at it just
by reading a usage site. It's not really improving the readability of
the code, it's in fact obscuring it as the reader has to actually look
up what the hell this means and then stumble upon a completely
undocumented lock guard define.

I'm all for using guards, but using guards just for the sake of using
guards is not a really good approach.

Thanks,

        tglx

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ