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

Kashyap Desai
2020-07-09T19:01:51Z

> >> If you want to share an updated megaraid sas driver patch based on
> >> that, then that's fine. I can incorporate that change in the patch
> >> where we add host_tagset to the scsi host template.
> > If you share git repo link of next submission, I can send you
> > megaraid_sas driver patch which you can include in series.
>
> So this is my work-en-progress branch:
>
> 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 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.
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.

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 ?

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 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.

>
> Thanks,
> John