Re: [patch V2 26/45] genirq/chip: Rework irq_set_handler() variants

Jiri Slaby
2025-05-12T07:25:24Z

On 11. 05. 25, 19:49, Nathan Chancellor wrote:
> On Sun, May 11, 2025 at 07:29:11PM +0200, Miguel Ojeda wrote:
>> On Fri, 09 May 2025 14:22:11 +0100 Nathan Chancellor <nathan@kernel.org> wrote:
>>>
>>> I am investigating some cases where
>>>
>>> WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>>>
>>> in start_kernel() in init/main.c is triggered in certain builds with
>>> clang after patch 23 of this series (very bizarre since the conversion
>>> seems to be correct) and I happened to notice that this conversion seems
>>> to be incorrect? Should this be scoped_irqdesc_get_and_buslock() like
>>> below?
>>
>> Yeah, I am also seeing this in next-20250509 in a LLVM=1 arm64 defconfig + Rust
>> build under QEMU.
>
> I noticed that the warning was reproducible with just the first patch of
> the series that adds the lock guards and patch 23 but also several other
> individual patches within the series, as I could not just revert patch
> 23 on next-20250509 to fix it. I have no idea why yet because I have not
> had the chance to actually sit down and dig into it but this diff fixes
> every instance of the warning that I saw in my tests... :/ could be a
> compiler bug or just some difference in behavior between compilers.
>
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index bd2db6ebb98e..94f463de8f26 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -176,10 +176,9 @@ __DEFINE_UNLOCK_GUARD(irqdesc_lock, struct irq_desc,
> static inline class_irqdesc_lock_t class_irqdesc_lock_constructor(unsigned int irq, bool bus,
> unsigned int check)
> {
> - class_irqdesc_lock_t _t = {
> - .bus = bus,
> - .lock = __irq_get_desc_lock(irq, &_t.flags, bus, check),

I assume the value stored by __irq_get_desc_lock() to &_t.flags is
overwritten by 0 by the initializer. class_irqdesc_lock_t::flags is
later than ::lock in the structure, so __irq_get_desc_lock() should be
called, setting ::flags, then the initializer should set flags to 0.

> - };
> + class_irqdesc_lock_t _t = {};
> + _t.bus = bus;
> + _t.lock = __irq_get_desc_lock(irq, &_t.flags, bus, check);

That's why this works ^^.

> return _t;
> }
>

--
js
suse labs