[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86jyzut2ni.wl-maz@kernel.org>
Date: Thu, 13 Nov 2025 14:42:25 +0000
From: Marc Zyngier <maz@...nel.org>
To: Luigi Rizzo <lrizzo@...gle.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
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 1/6] genirq: platform wide interrupt moderation: Documentation, Kconfig, irq_desc
On Thu, 13 Nov 2025 13:33:51 +0000,
Luigi Rizzo <lrizzo@...gle.com> wrote:
>
> On Thu, Nov 13, 2025 at 2:25 PM Marc Zyngier <maz@...nel.org> wrote:
> >
> > On Wed, 12 Nov 2025 19:24:03 +0000,
> > Luigi Rizzo <lrizzo@...gle.com> wrote:
> >
> > [...]
> >
> > > The system does not rely on any special hardware feature except from
> > > MSI-X Pending Bit Array (PBA), a mandatory component of MSI-X
> >
> > Is this stuff PCI specific? if so, Why? What is the actual dependency
> > on PBA? It is it just that you are relying on the ability to mask
> > interrupts without losing them, something that is pretty much a given
> > on any architecture?
>
> You are right, I was overly restrictive. I only need what you say,
> will replace the text accordingly.
>
> >
> > [...]
> > > +To understand the motivation for this feature, we start with some
> > > +background on interrupt moderation.
> >
> > This reads like marketing blurb. This is an API documentation, and it
> > shouldn't be a description of your motivations for building it the way
> > you did. I'd suggest you stick to the API, and keep the motivations
> > for the cover letter.
>
> ok will remove it.
> Sorry if it reads like marketing, that is very very far from my intentions.
> I just wanted to give background information in a way that is easy
> to access from the source tree.
Background information is only valid at a given point in time, for a
particular configuration, and rarely translate into something that
spans architectures and implementation in a universal way. For a
start, the quoted figures for interrupt rates are pretty much
irrelevant -- think of reading this in 10 years...
The descriptions are also massively x86-specific. That's probably OK
for the stuff you care about, but I'd certainly would want things to
be a bit more abstract and applicable to all architectures.
> > > +* **Interrupt** is a mechanism to **notify** the CPU of **events**
> > > + that should be handled by software, for example, **completions**
> > > + of I/O requests (network tx/rx, disk read/writes...).
> >
> > That's only half of the truth, as this description only applies to
> > *edge* interrupts. Level interrupts report a change in *state*, not an
> > event.
> >
> > How do you plan to deal with interrupt moderation for level
> > interrupts?
>
> I don't. This is restricted to edge interrupts.
I also note that since you explicitly check for handle_edge_irq() in
set_moderation_mode(), this will not work on anything GIC related, or
any other architecture that uses the fasteoi flows. I really wonder
why you are not looking at the actual trigger mode instead...
Until you fix it, please refrain from touching the GICv3 code, and
make sure this is solely enabled on x86 -- it clearly wasn't tested on
anything else.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists