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;
Date:   Fri, 07 Apr 2023 10:18:20 +0100
Message-ID: <86lej4uk2r.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Radu Rendec <rrendec@redhat.com>
Cc:     Marek =?UTF-8?B?QmVow7pu?= <kabel@kernel.org>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        pali@kernel.org, Brian Masney <bmasney@redhat.com>
Subject: Re: irqdomain API: how to set affinity of parent irq of chained irqs?
In-Reply-To: <d1166229fdead30fcbf3c2bd88ca1bd4ebffbce9.camel@redhat.com>
References: <20220502102137.764606ee@thinkpad>
        <87mtg0i8m8.wl-maz@kernel.org>
        <20220502174559.78f5cbc0@dellmb>
        <87fslr7ygl.wl-maz@kernel.org>
        <d1166229fdead30fcbf3c2bd88ca1bd4ebffbce9.camel@redhat.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Radu,

On Fri, 07 Apr 2023 00:56:40 +0100,
Radu Rendec <rrendec@redhat.com> wrote:
>=20
> Hello Marc, Marek,
>=20
> On Tue, 2022-05-03 at 10:32 +0100, Marc Zyngier wrote:
> > On Mon, 02 May 2022 16:45:59 +0100,
> > Marek Beh=C3=BAn <kabel@kernel.org> wrote:
> > >=20
> > > On Mon, 02 May 2022 10:31:11 +0100
> > > Marc Zyngier <maz@kernel.org> wrote:
> > >=20
> > > > On Mon, 02 May 2022 09:21:37 +0100,
> > > > Marek Beh=C3=BAn <kabel@kernel.org> wrote:
> > > > >=20
> > > > > Dear Marc, Thomas,
> > > > >=20
> > > > > we have encountered the following problem that can hopefully be p=
ut
> > > > > some light onto: What is the intended way to set affinity (and po=
ssibly
> > > > > other irq attributes) of parent IRQ of chained IRQs, when using t=
he
> > > > > irqdomain API?=C2=A0=20
> > > >=20
> > > > Simples: you can't. What sense does it make to change the affinity =
of
> > > > the parent interrupt, given that its fate is tied to *all* of the
> > > > other interrupts that are muxed to it?
> > >=20
> > > Dear Marc,
> > >=20
> > > thank you for your answer. Still:
> > >=20
> > > What about when we want to set the same affinity for all the chained
> > > interrupts?
> > >=20
> > > Example: on Armada 385 there are 4 PCIe controllers. Each controller
> > > has one interrupt from which we trigger chained interrupts. We would
> > > like to configure each controller to trigger interrupt (and thus all
> > > chained interrupts in the domain) on different CPU core.
> > >=20
> > > Moreover we would really like to do this in runtime, through sysfs,
> > > depending on for example whether there are cards plugged in the PCIe
> > > ports.
> > >=20
> > > Maybe there should be some mechanism to allow to change affinity for
> > > whole irqdomain, or something?
> >=20
> > Should? Maybe. But not for an irqdomain (which really doesn't have
> > anything to do with interrupt affinity).
> >=20
> > What you may want is a new sysfs interface that would allow a parent
> > interrupt affinity being changed, but also exposing to userspace all
> > the interrupts this affects *at the same time*. something like:
> >=20
> > /sys/kernel/irq/42/smp_affinity_list
> > /sys/kernel/irq/42/muxed_irqs/
> > /sys/kernel/irq/42/muxed_irqs/56 -> ../../56
> > /sys/kernel/irq/42/muxed_irqs/57 -> ../../57
> >=20
> > The main issues are that:
> >=20
> > - we don't really track the muxing information in any of the data
> > =C2=A0 structures, so you can't just walk a short list and generate this
> > =C2=A0 information. You'd need to build the topology information at
> > =C2=A0 allocation time (or fish it out at runtime, but that's likely a
> > =C2=A0 pain).
> >=20
> > - sysfs doesn't deal with affinities at all. procfs does, but adding
> > =C2=A0 more crap there is frowned upon.
> >=20
> > - it *must* be a new interface. You can't repurpose the existing one,
> > =C2=A0 as something like irqbalance would be otherwise be massively
> > =C2=A0 confused by seeing interrupts moving around behind its back.
> >=20
> > - conversely, you'll need to teach irqbalance how to deal with this
> > =C2=A0 new interface.
> >=20
> > - this needs to be safe against CPU hotplug. It probably already is,
> > =C2=A0 but nobody ever tested it, given that userspace can't interact w=
ith
> > =C2=A0 these interrupts at the moment.
>=20
> Are you aware of any work being done (or having been done) in this
> area? Thanks in advance!
>=20
> My colleagues and I are looking into picking this up and implementing
> the new sysfs interface and the related irqbalance changes, and we are
> currently evaluating the level of effort. Obviously, we would like to
> avoid any effort duplication.

I don't think anyone ever tried it (it's far easier to just moan about
it than to do anything useful). But if you want to start looking into
that, that'd be great.

One of my concern is that allowing affinity changes for chained
interrupt may uncover issues in existing drivers, so it would have to
be an explicit buy-in for any chained irqchip. That's probably not too
hard to achieve anyway given that you'll need some new infrastructure
to track the muxed interrupts.

Hopefully this will result in something actually happening! ;-)

Cheers,

	M.

--=20
Without deviation from the norm, progress is not possible.