linux-kernel - Re: [PATCH 3/6] genirq: soft_moderation: activate hooks in handle_irq

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: <CAMOZA0Kak5bqEQytGQieHY1r6Loo-pAZks8qkROjU5z+OD2=HA@mail.gmail.com>
Date: Fri, 14 Nov 2025 09:27:39 +0100
From: Luigi Rizzo <lrizzo@...gle.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Marc Zyngier <maz@...nel.org>, Luigi Rizzo <rizzo.unipi@...il.com>, 
	Paolo Abeni <pabeni@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Sean Christopherson <seanjc@...gle.com>, Jacob Pan <jacob.jun.pan@...ux.intel.com>, 
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH 3/6] genirq: soft_moderation: activate hooks in handle_irq_event()

On Thu, Nov 13, 2025 at 10:45 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Wed, Nov 12 2025 at 19:24, Luigi Rizzo wrote:
> >  [...]
> > @@ -254,9 +255,11 @@ irqreturn_t handle_irq_event(struct irq_desc *desc)
> >       irqd_set(&desc->irq_data, IRQD_IRQ_INPROGRESS);
> >       raw_spin_unlock(&desc->lock);
> >
> > +     irq_moderation_hook(desc); /* may disable irq so must run unlocked */
>
> That's just wrong. That can trivially be implemented in a way which
> works with the lock held.

Do you mean I can move the call before the unlock and use __disable_irq()
given that  moderation is only enabled on interrupts that have
irq_bus_lock == NULL, so disable_irq_nosync() which is
        scoped_irqdesc_get_and_buslock(irq, IRQ_GET_DESC_CHECK_GLOBAL) {
                __disable_irq(scoped_irqdesc);
                return 0;
        }
effectively collapses to raw_spin_lock()
This would save a lock/unlock dance and a few conditionals?

> > @@ -134,6 +134,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
> >       desc->tot_count = 0;
> >       desc->name = NULL;
> >       desc->owner = owner;
> > +     irq_moderation_init_fields(desc);
>
> That's clearly part of activation in handle_irq_event() ....

I don't follow. As far as I understand desc_set_defaults() makes sure we
have a clean descriptor after release/before allocation. This includes
clearing the moderation enable flag from a previous user of this desc,
and the flag is only read in the hook in handle_irq_event().

cheers
luigi