linux-kernel - RE: [RFC 11/12] Drivers: hv: vmbus: Wait for MODIFYCHANNEL to finish when offlining CPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 24 Jun 2024 19:32:48 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Boqun Feng <boqun.feng@...il.com>
CC: "kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
	<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
	"decui@...rosoft.com" <decui@...rosoft.com>, "tglx@...utronix.de"
	<tglx@...utronix.de>, "mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de"
	<bp@...en8.de>, "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "kw@...ux.com"
	<kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "James.Bottomley@...senpartnership.com"
	<James.Bottomley@...senpartnership.com>, "martin.petersen@...cle.com"
	<martin.petersen@...cle.com>, "arnd@...db.de" <arnd@...db.de>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>, "maz@...nel.org"
	<maz@...nel.org>, "den@...inux.co.jp" <den@...inux.co.jp>,
	"jgowans@...zon.com" <jgowans@...zon.com>, "dawei.li@...ngroup.cn"
	<dawei.li@...ngroup.cn>
Subject: RE: [RFC 11/12] Drivers: hv: vmbus: Wait for MODIFYCHANNEL to finish
 when offlining CPUs

From: Boqun Feng <boqun.feng@...il.com> Sent: Monday, June 24, 2024 10:55 AM
> 
> Hi Michael,
> 
> On Mon, Jun 03, 2024 at 10:09:39PM -0700, mhkelley58@...il.com wrote:
> [...]
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index bf35bb40c55e..571b2955b38e 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -264,6 +264,14 @@ struct vmbus_connection {
> >  	struct irq_domain *vmbus_irq_domain;
> >  	struct irq_chip	vmbus_irq_chip;
> >
> > +	/*
> > +	 * VM-wide counts of MODIFYCHANNEL messages sent and completed.
> > +	 * Used when taking a CPU offline to make sure the relevant
> > +	 * MODIFYCHANNEL messages have been completed.
> > +	 */
> > +	u64 modchan_sent;
> > +	u64 modchan_completed;
> > +
> 
> Looks to me, we can just use atomic64_t here: modifying channels is far
> from hotpath, so the cost of atomic increment is not a big issue, and we
> avoid possible data races now and in the future.
> 
> Thoughts?

At one point in the development, I did have these as atomic64_t.
And I agree that the usage is pretty infrequent, so the atomic costs
aren't an issue. But both fields are already safe. modchan_sent
is covered by vmbus_connection.set_affinity_lock as held in
vmbus_irq_set_affinity(). And modchan_completed is OK because
it is only ever incremented by code running on VMBUS_CONNECT_CPU,
per the comment in vmbus_onmodifychannel_response().

Converting to atomic64_t wouldn't hurt anything, and arguably
would provide additional robustness, but it just didn't seem
necessary.

Michael