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

[John Garry <john.garry@huawei.com>]

>>
>> https://github.com/hisilicon/kernel-dev/commits/private-topic-blk-mq-
>> shared-tags-rfc-v8
> I tested this repo + megaraid_sas shared hosttag driver. This repo (5.8-rc)
> has CPU hotplug patch.
> " bf0beec0607d blk-mq: drain I/O when all CPUs in a hctx are offline"
> 
> 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?

  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;

> 
> megaraid_sas driver has  internal reply_queue mapping which helps to get IO
> completion on same cpu.  Driver get msix index from that table based on "
> raw_smp_processor_id".
> If table is mapped correctly at probe time,  It is not possible to pick
> entry of offline CPU.
> 
> Am I missing anything ?

Not sure, I think I need to be clear exactly what you're doing.

> 
> If you can help me to understand why we need shared host tag for CPU
> hotplug, I can try to frame some test case for possible reproduction.

I think it's best explained in cover letter for "blk-mq: Facilitate a 
shared sbitmap per tagset".

See points "HBA HW queues are required to be mapped to that of the 
blk-mq hctx", "HBA LLDD would have to generate this tag internally", and 
"blk-mq assumes the host may accept (Scsi_host.can_queue * #hw queue) 
commands".
> 
>> I just updated to include the change to have Scsi_Host.host_tagset in
>> 4291f617a02b commit ("scsi: Add host and host template flag
>> 'host_tagset'")
>>
>> megaraid sas support is not on the branch yet, but I think everything else
>> required is. And it is mutable, so I'd clone it now if I were you - or
>> just replace
>> the required patch onto your v7 branch.
> I am working on this.
> 

Great, thanks