Re: [PATCH 3/6] genirq: soft_moderation: activate hooks in handle_irq_event()

[Luigi Rizzo]

On Thu, Nov 13, 2025 at 10:45 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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