linux-kernel - RE: [PATCH] scsi: storvsc: Prefer returning channel with the same CPU as on the I/O issuing CPU

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <DS3PR21MB573566DF7A81D555552DE8A7CEE1A@DS3PR21MB5735.namprd21.prod.outlook.com>
Date: Wed, 8 Oct 2025 00:55:57 +0000
From: Long Li <longli@...rosoft.com>
To: Michael Kelley <mhklinux@...look.com>, "longli@...ux.microsoft.com"
	<longli@...ux.microsoft.com>, KY Srinivasan <kys@...rosoft.com>, Haiyang
 Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>, "James E.J. Bottomley"
	<James.Bottomley@...senPartnership.com>, "Martin K. Petersen"
	<martin.petersen@...cle.com>, James Bottomley <JBottomley@...n.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] scsi: storvsc: Prefer returning channel with the same CPU
 as on the I/O issuing CPU



> -----Original Message-----
> From: Michael Kelley <mhklinux@...look.com>
> Sent: Tuesday, October 7, 2025 8:42 AM
> To: longli@...ux.microsoft.com; KY Srinivasan <kys@...rosoft.com>; Haiyang
> Zhang <haiyangz@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> <decui@...rosoft.com>; James E.J. Bottomley
> <James.Bottomley@...senPartnership.com>; Martin K. Petersen
> <martin.petersen@...cle.com>; James Bottomley <JBottomley@...n.com>;
> linux-hyperv@...r.kernel.org; linux-scsi@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Cc: Long Li <longli@...rosoft.com>
> Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning channel with the
> same CPU as on the I/O issuing CPU
> 
> From: longli@...ux.microsoft.com <longli@...ux.microsoft.com> Sent:
> Wednesday, October 1, 2025 10:06 PM
> >
> > When selecting an outgoing channel for I/O, storvsc tries to select a
> > channel with a returning CPU that is not the same as issuing CPU. This
> > worked well in the past, however it doesn't work well when the Hyper-V
> > exposes a large number of channels (up to the number of all CPUs). Use
> > a different CPU for returning channel is not efficient on Hyper-V.
> >
> > Change this behavior by preferring to the channel with the same CPU as
> > the current I/O issuing CPU whenever possible.
> >
> > Tests have shown improvements in newer Hyper-V/Azure environment, and
> > no regression with older Hyper-V/Azure environments.
> >
> > Tested-by: Raheel Abdul Faizy <rabdulfaizy@...rosoft.com>
> > Signed-off-by: Long Li <longli@...rosoft.com>
> > ---
> >  drivers/scsi/storvsc_drv.c | 96
> > ++++++++++++++++++--------------------
> >  1 file changed, 45 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index d9e59204a9c3..092939791ea0 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1406,14 +1406,19 @@ static struct vmbus_channel *get_og_chn(struct
> storvsc_device *stor_device,
> >  	}
> >
> >  	/*
> > -	 * Our channel array is sparsley populated and we
> > +	 * Our channel array could be sparsley populated and we
> >  	 * initiated I/O on a processor/hw-q that does not
> >  	 * currently have a designated channel. Fix this.
> >  	 * The strategy is simple:
> > -	 * I. Ensure NUMA locality
> > -	 * II. Distribute evenly (best effort)
> > +	 * I. Prefer the channel associated with the current CPU
> > +	 * II. Ensure NUMA locality
> > +	 * III. Distribute evenly (best effort)
> >  	 */
> >
> > +	/* Prefer the channel on the I/O issuing processor/hw-q */
> > +	if (cpumask_test_cpu(q_num, &stor_device->alloced_cpus))
> > +		return stor_device->stor_chns[q_num];
> > +
> 
> Hmmm. When get_og_chn() is called, we know that stor_device-
> >stor_chns[q_num] is NULL since storvsc_do_io() has already handled the non-
> NULL case. And the checks are all done with stor_device->lock held, so the
> stor_chns array can't change.
> Hence the above code will return NULL, which will cause a NULL reference when
> storvsc_do_io() sends out the VMBus packet.
> 
> My recollection is that get_og_chan() is called when there is no channel that
> interrupts the current CPU (that's what it means for stor_device-
> >stor_chns[<current CPU>] to be NULL). So the algorithm must pick a channel
> that interrupts some other CPU, preferably a CPU in the current NUMA node.
> Adding code to prefer the channel associated with the current CPU doesn't make
> sense in get_og_chn(), as get_og_chn() is only called when it is already known
> that there is no such channel.

The initial values for stor_chns[] and alloced_cpus are set in storvsc_channel_init() (for primary channel) and handle_sc_creation() (for subchannels).

As a result, the check for cpumask_test_cpu(q_num, &stor_device->alloced_cpus) will guarantee we are getting a channel. If the check fails, the code follows the old behavior to find a channel.

This check is needed because storvsc supports change_target_cpu_callback() callback via vmbus.

Thanks,

Long

> 
> Or is there a case that I'm missing? Regardless, the above code seems
> problematic because it would return NULL.
> 
> Michael