Received-SPF: pass (google.com: domain of linux-kernel+bounces-202731-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Michael Kelley <mhklinux@outlook.com>
To: Thomas Gleixner <tglx@linutronix.de>, "kys@microsoft.com"
	<kys@microsoft.com>, "haiyangz@microsoft.com" <haiyangz@microsoft.com>,
	"wei.liu@kernel.org" <wei.liu@kernel.org>, "decui@microsoft.com"
	<decui@microsoft.com>, "mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de"
	<bp@alien8.de>, "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>,
	"lpieralisi@kernel.org" <lpieralisi@kernel.org>, "kw@linux.com"
	<kw@linux.com>, "robh@kernel.org" <robh@kernel.org>, "bhelgaas@google.com"
	<bhelgaas@google.com>, "James.Bottomley@HansenPartnership.com"
	<James.Bottomley@HansenPartnership.com>, "martin.petersen@oracle.com"
	<martin.petersen@oracle.com>, "arnd@arndb.de" <arnd@arndb.de>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
CC: "maz@kernel.org" <maz@kernel.org>, "den@valinux.co.jp"
	<den@valinux.co.jp>, "jgowans@amazon.com" <jgowans@amazon.com>,
	"dawei.li@shingroup.cn" <dawei.li@shingroup.cn>
Subject: RE: [RFC 06/12] genirq: Add per-cpu flow handler with conditional IRQ
 stats
Thread-Topic: [RFC 06/12] genirq: Add per-cpu flow handler with conditional
 IRQ stats
Thread-Index: AQHatj3dxfDAEhKlmk6xJJhdAubqnrG36WaAgAAakqCAASXCgIAAA1YQ
Date: Wed, 5 Jun 2024 13:45:26 +0000
Message-ID:
 <SN6PR02MB415706390CB0E8FD599B6494D4F92@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240604050940.859909-1-mhklinux@outlook.com>
 <20240604050940.859909-7-mhklinux@outlook.com> <87h6e860f8.ffs@tglx>
 <SN6PR02MB415737FF6F7B40A1CD20C4A9D4F82@SN6PR02MB4157.namprd02.prod.outlook.com>
 <87zfrz4jce.ffs@tglx>
In-Reply-To: <87zfrz4jce.ffs@tglx>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?katvRQVerasrzsGDz5nbYaZpD0OLhUMZ1EDzoT6ak6G2q0wd12OHW5QazrYH?=
 =?us-ascii?Q?DJn686n4LzIBSyoQqaut4MQNwUV083rJsmXEHfHARow1O8uvNF84WUL8aB9z?=
 =?us-ascii?Q?JQcJr7HNi67wiJ4x5jjoRwdUoi69YhgnceMMyJeozFK0Tk1p30L2SdHIN+bJ?=
 =?us-ascii?Q?XOgIt+432VEdMOIH4tFX96UNExBg2MszOrJubznmo8DhigT5+zgoUQqEgljU?=
 =?us-ascii?Q?AUDr+kYZ+22Hwwo1Bf9rVB+fXN79WU9hnSSnfOaneHAe44K4JsXpESmbX6yX?=
 =?us-ascii?Q?raYSJK7cPhpIhLzbuoGE7RaPYLwT9jBGTVPQz0GKRgn03OKUT4cl8gyS+qNK?=
 =?us-ascii?Q?bnq/PSwUV8lUpJaW9cycQQeJUx046A4XVNBdZOgktxCK+wXKc2EfmWQZxL0Z?=
 =?us-ascii?Q?KgAFyKCLO50DDGFy3cF73M4s2SBtTn0RjJhN4yKaZhYzwqGaTVF/DLuoUS85?=
 =?us-ascii?Q?oMzuumz4WoXVe3a+VSmSQjUEwypDu/AcdsxFrhnd4Etir3zDz0sXmybgcd1L?=
 =?us-ascii?Q?/cRvcmsRK5YIltTdOZJ2qm8ys5vqSX9eHI7ynSVTRisQ9z8RlpxOYSG0d3s4?=
 =?us-ascii?Q?vceTSLkQKZbCd4gxFYmjJ7X72XYwKogQF4gjJfDHnNIwlFthSXTC2C7ni1zd?=
 =?us-ascii?Q?b2fWdMA2J7me8kml6lc8Uizdk8UNf3qjdbgLa814l3HplSSu7L2budM+62bq?=
 =?us-ascii?Q?W5ErUG1g1FWV2IFu2JI1SRw1WVq+d4dKtWit50lR41UL4v3comT8MT222JrB?=
 =?us-ascii?Q?HoZPz1wXd3nrFhFkSkm258sROJ2T6b7+tFJKsiw0U365AceWrCvHBzQtOiVS?=
 =?us-ascii?Q?K7gq+0PCUykrvKIQpkH1tDagHzk92hIW9an5oNAt3FhNzvXJM48fPLEDVtJZ?=
 =?us-ascii?Q?cG1R+Sm0qSj4KlJOSYGzMuvrugH7Y/y1xmoYuKi/T4Gs4HdC24Ef3rerQfJm?=
 =?us-ascii?Q?LctWhxDYxATvzoVS1kYZcFSJW1y0208GatFwNVTUDhd7o16CtmfPxelfIjUi?=
 =?us-ascii?Q?/VtmwOfqNacE1fWU+KfONPe8rER/apnqQ6mMY9QMsP2Mcgq9x3MDuCVS0EtY?=
 =?us-ascii?Q?PsysZsWNEcrjTZjvLot4/ahS+b/nOPotXSEMLZaIUlaXCW6VMJEzciz5D2kf?=
 =?us-ascii?Q?uW/Re55ycLCh6LQnRj/saj8JKzLVSOz6V4x+p5cx3tVymzGn3MEdlnGGG3Fx?=
 =?us-ascii?Q?Qf+9V9/kvNPycLDkqdRLcBSv+Yf1TlEcuQHW8a/7l6lR9NmO+IYP4CvhC8Y?=
 =?us-ascii?Q?=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 7b239599-1894-457a-fddd-08dc8565c167
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Jun 2024 13:45:26.2729
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR02MB7778

From: Thomas Gleixner <tglx@linutronix.de> Sent: Wednesday, June 5, 2024 6:=
20 AM
>=20
> On Tue, Jun 04 2024 at 23:03, Michael Kelley wrote:
> > From: Thomas Gleixner <tglx@linutronix.de> Sent: Tuesday, June 4, 2024 =
11:14 AM
> >>    1) Move the inner workings of handle_percpu_irq() out into
> >>       a static function which returns the 'handled' value and
> >>       share it between the two handler functions.
> >
> > The "inner workings" aren't quite the same in the two cases.
> > handle_percpu_irq() uses handle_irq_event_percpu() while
> > handle_percpu_demux_irq() uses __handle_irq_event_percpu().
> > The latter doesn't do add_interrupt_randomness() because the
> > demultiplexed IRQ handler will do it.  Doing add_interrupt_randomness()
> > twice doesn't break anything, but it's more overhead in the hard irq
> > path, which I'm trying to avoid.  The extra functionality in the
> > non-double-underscore version could be hoisted up to
> > handle_percpu_irq(), but that offsets gains from sharing the
> > inner workings.
>=20
> That's not rocket science to solve:
>=20
> static irqreturn_t helper(desc, func)
> {
> 	boiler_plate..
>         ret =3D func(desc)
> 	boiler_plate..
>         return ret;
> }
>=20
> No?
>=20
> TBH, I still hate that conditional accounting :)
>=20
> >>    2) Allocate a proper interrupt for the management mode and invoke i=
t
> >>       via generic_handle_irq() just as any other demultiplex interrupt=
.
> >>       That spares all the special casing in the core code and just
> >>       works.
> >
> > Yes, this would work on x86, as the top-level interrupt isn't a Linux I=
RQ,
> > and the interrupt counting is done in Hyper-V specific code that could =
be
> > removed.  The demux'ed interrupt does the counting.
> >
> > But on arm64 the top-level interrupt *is* a Linux IRQ, so each
> > interrupt will get double-counted, which is a problem.
>=20
> What is the problem?
>=20
> You have: toplevel, mgmt, device[], right?
>=20
> They are all accounted for seperately and each toplevel interrupt might
> result in demultiplexing one or more interrupts (mgmt, device[]), no?
>=20
> IMO accounting the toplevel interrupt seperately is informative because
> it allows you to figure out whether demultiplexing is clustered or not,
> but I lost that argument long ago. That's why most demultiplex muck
> installs a chained handler, which is a design fail on it's own.

In /proc/interrupts, the double-counting isn't a problem, and is
potentially helpful as you say. But /proc/stat, for example, shows a total
interrupt count, which will be roughly double what it was before. That
/proc/stat value then shows up in user space in vmstat, for example.
That's what I was concerned about, though it's not a huge problem in
the grand scheme of things.

Michael