RE: [PATCH RFC v7 10/12] megaraid_sas: switch fusion adapters to MQ

From: Kashyap Desai <kashyap.desai@broadcom.com>
To: John Garry <john.garry@huawei.com>,
	axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com,
	don.brace@microsemi.com, Sumit Saxena <sumit.saxena@broadcom.com>,
	ming.lei@redhat.com, bvanassche@acm.org, hare@suse.com,
	hch@lst.de,
	Shivasharan Srikanteshwara 
	<shivasharan.srikanteshwara@broadcom.com>
Cc: linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	esc.storagedev@microsemi.com, chenxiang66@hisilicon.com,
	"PDL,MEGARAIDLINUX" <megaraidlinux.pdl@broadcom.com>
Subject: RE: [PATCH RFC v7 10/12] megaraid_sas: switch fusion adapters to MQ
Date: Mon, 13 Jul 2020 13:25:36 +0530	[thread overview]
Message-ID: <34a832717fef4702b143ea21aa12b79e@mail.gmail.com> (raw)
In-Reply-To: <13d6b63e-3aa8-68fa-29ab-a4c202024280@huawei.com>

> > Looking at description of above patch and changes, it looks like
> > megaraid_sas driver can still work without shared host tag for this
> > feature.
> >
> > I observe CPU hotplug works irrespective of shared host tag
>
> Can you be clear exactly what you mean by "irrespective of shared host
> tag"?
>
> Do you mean that for your test Scsi_Host.nr_hw_queues is set to expose hw
> queues and scsi_host_template.map_queues = blk_mq_pci_map_queues(),
> but you just don't set the host_tagset flag?

Yes. I only disabled "host_tagset". <map_queue> is still hooked.

>
>   in megaraid_sas
> > on 5.8-rc.
> >
> > Without shared host tag, megaraid driver will expose single hctx and
> > all the CPU will be mapped to hctx0.
>
> right
>
> > Any CPU offline event will have " blk_mq_hctx_notify_offline" callback
> > in blk-mq module. If we do not have this callback/patch, we will see
> > IO timeout.
> > blk_mq_hctx_notify_offline callback will make sure all the outstanding
> > on
> > hctx0 is cleared and only after it is cleared, CPU will go offline.
>
> But that is only for when the last CPU for the hctx is going offline. If
> nr_hw_queues == 1, then hctx0 would cover all CPUs, so that would never
> occur during normal operation. See initial check in
> blk_mq_hctx_notify_offline():
>
> static int blk_mq_hctx_notify_offline(unsigned int cpu, struct hlist_node
> *node) {
> 	if (!cpumask_test_cpu(cpu, hctx->cpumask) ||
> 	    !blk_mq_last_cpu_in_hctx(cpu, hctx))
> 		return 0;
>

Thanks John for this pointer. I missed this part and now I understood what
was happening in my testing.
There were more than one CPU mapped to one msix index in my earlier testing
and because of that I could see Interrupt migration happens on available CPU
from affinity mask. So my earlier testing was incorrect.

Now I am consistently able to reproduce issue - Best setup is have 1:1
mapping of CPU to MSIX vector mapping. I used 128 logical CPU and 128 msix
vectors and I noticed IO timeout without this RFC (without host_tagset).
I did not noticed IO timeout with RFC (with host_tagset.) I will update this
data in Driver's commit message.

Just for my understanding -
What if we have below code in blk_mq_hctx_notify_offline, CPU hotplug should
work for megaraid_sas driver without this RFC (without shared host tagset).
Right ?
If answer is yes, will there be any side effect of having below code in
block layer ?

static int blk_mq_hctx_notify_offline(unsigned int cpu, struct hlist_node
 *node) {
 	if (hctx->queue->nr_hw_queues > 1
	    && (!cpumask_test_cpu(cpu, hctx->cpumask) ||
 	    !blk_mq_last_cpu_in_hctx(cpu, hctx)))
 		return 0;

I also noticed nr_hw_queues are now exposed in sysfs -

/sys/devices/pci0000:85/0000:85:00.0/0000:86:00.0/0000:87:04.0/0000:8b:00.0/0000:8c:00.0/0000:8d:00.0/host14/scsi_host/host14/nr_hw_queues:128