Re: [PATCH 2/6] genirq: soft_moderation: add base files, procfs hooks

[Luigi Rizzo]

On Thu, Nov 13, 2025 at 10:29 AM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> On Wed, Nov 12 2025 at 19:24, Luigi Rizzo wrote:
> >  [...]

most other comments will be addressed in the next version, but I wanted
to address a few of them here:

> > /sys/module/irq_moderation/parameters and read/write the procfs entries
> > /proc/irq/soft_moderation and /proc/irq/NN/soft_moderation.
> >
> > Examples:
> > cat /proc/irq/soft_moderation
> > echo "delay_us=345" > /proc/irq/soft_moderation
> > echo 1 | tee /proc/irq/*/nvme*/../soft_moderation
>
> That's impressive to be able to write into proc/...

That is already the case for /proc/irq/default_smp_affinity and
/proc/irq/NN/smp_affinity[_list] so I followed the same approach.

> > +
> > +/* After the handler, if desc is moderated, make sure the timer is active. */
> > +static inline void irq_moderation_epilogue(const struct irq_desc *desc)
> > +{
> > +     struct irq_mod_state *ms = this_cpu_ptr(&irq_mod_state);
>
> Lacks a check whether moderation is enabled or not.

That would be redundant, !list_empty(desc->ms_node) is only true
if moderation is enabled and many other conditions in the previous hook.

> > +             /* Timer still alive. Just call the handlers */
> > +             list_for_each_entry_safe(desc, next, &ms->descs, ms_node) {
> > +                     ms->irq_count += irq_mod_info.count_timer_calls;
> > +                     ms->timer_calls++;
> > +                     handle_irq_event_percpu(desc);
>
> That lacks setting the INPROGRESS flag.

I did it in an earlier test version, but was wondering if that is
actually needed.
While the irqdesc is in the timer list, irqd_irq_disabled() is true and that
makes irq_can_handle_actions() and irq_can_handle() return false.

> > +#pragma clang diagnostic error "-Wformat"
>
> What's that for?

print() formats with a lot of entries are error prone, and the above
catches a lot of trivial mistakes, so I felt it was useful to keep it.

cheers
luigi