Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Radu Rendec <rrendec@redhat.com>, linux-kernel@vger.kernel.org
Cc:     Marc Zyngier <maz@kernel.org>
Subject: Re: [RFC PATCH 0/5] irq: sysfs interface improvements for SMP
 affinity control
In-Reply-To: <20230530214550.864894-1-rrendec@redhat.com>
Date:   Wed, 31 May 2023 15:09:10 +0200
Message-ID: <877csohcll.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Tue, May 30 2023 at 17:45, Radu Rendec wrote:
> Note: I still need to update the Documentation/ directory for the new
>       sysfs interface, and I will address that in a future version.
>       At this point, I just want to get feedback about the current
>       approach.

From a conceptual POV I understand why this is required, which makes me
hate this chained mechanism even more.

Aside of having no visibility (counters, affinity, etc) the worst thing
about these chained hidden interrupts is:

  There is no control of runaway interrupts as they circumvent the core,
  which has caused hard to diagnose system hangups in the past. See
   
    ba714a9c1dea ("pinctrl/amd: Use regular interrupt instead of chained")

  for demonstration.

The argument I heard for this chained interrupt muck is that it's so
much more performant than using regular interrupt handlers via
request_irq(). It's obviously less overhead, but whether it matters and
most importantly whether it justifies the downsides is a different
question.

There is also the argument about double accounting. Right now the
chained handler is not accounted and only the childs are.

Though that is inconsistent with other demultiplex handlers which _must_
use regular interrupt handlers (threaded) to demultiplex interrupt chips
which sit behind SPI/I2C...

The sum of child interrupts is also not necessarily the number of parent
interrupts, unless there is always exactly only one child handler to
invoke.

Quite some of those demux handlers are also not RT compatible.

AFAICT, there is no real technical reason at least not for the vast
majority of usage sites why the demultiplex handler cannot run inside a
regular interrupt handler context.

So I personally would prefer to get rid of this chained oddball and just
have consistent mechanisms for dealing with interrupts, which would just
avoid exposing the affinity files in two different places.

Providing information about child/parent relationship is an orthogonal
issue.

If there is some good reason (aside of the chained muck) to have sysfs
based affinity management, then I'm not objecting as long as the
functionality is the same, i.e. effective affinity needs be exposed too.

Thanks,

        tglx