RE: [PATCH] blk-mq: avoid to iterate over stale request

[Kashyap Desai <kashyap.desai@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]

> >
> > shared_tags->rqs[5] of hctx0 is having scmd = 0xAA (inflight command)
> > shared_tags->rqs[10] of hctx0 is having scmd = 0xAA (inflight command)
> > <- This is incorrect. While looping on hctx0 tags[], bitmap = 10 this
> > entry is also found which is actually outstanding on hctx1.
> > shared_tags->rqs[10] of hctx1 is having scmd = 0xBB (inflight command)
>
> Sorry, I am a bit confused, please look at the following code and
> blk_mq_find_and_get_req()(<-bt_tags_iter).


My issue is without shared tags. Below patch is certainly a fix for shared
tags (for 5.16-rc).
I have seen scsi eh deadlock issue on 5.15 kernel which does not have
shared tags (but it has shared bitmap.)

>
> void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
>                 busy_tag_iter_fn *fn, void *priv) {
>         unsigned int flags = tagset->flags;
>         int i, nr_tags;
>
>         nr_tags = blk_mq_is_shared_tags(flags) ? 1 :
tagset->nr_hw_queues;
>
>         for (i = 0; i < nr_tags; i++) {
>                 if (tagset->tags && tagset->tags[i])
>                         __blk_mq_all_tag_iter(tagset->tags[i], fn, priv,
>                                               BT_TAG_ITER_STARTED);
>         }
> }
>
> In case of shared tags, only tagset->tags[0] is iterated over, so both
> 5 and 10 are checked, and shared_tags->rqs[5] and shared_tags->rqs[10]
> shouldn't just point to the two latest requests? How can both 0xAA &
0xBB be
> retrieved from shared_tags->rqs[10]?

Since I am not talking about shared tags, you can remap same condition
now.
In 5.15 kernel (shared bitmap is enabled but not shared tags) -
blk_mq_tagset_busy_iter is called for each tags[] of nr_hw_queues times.
It will call  bt_tags_for_each()  for hctx0.tags->[], hctx1.tags->[] ..
Since tags->bitmap_tags is shared when it traverse hctx0.tags->[], it can
find hctx0.tags[10].rq which is really stale entry.
If the same request which is stale @ hctx0.tags[10] is really outstanding
at some other tag#, stale entry will be counted in host_busy.

>
> > Issue noticed by me is the exact same issue described @ below -
> > https://lore.kernel.org/linux-scsi/fe5cf6c4-ce5e-4a0f-f4ab-5c10539492c
> > b@hu
> > awei.com/
> >
> > Issue is only exposed to shared host tagset. I got the required
> > information. Thanks.
> >
> > Kashyap
> > >
> > > > count total 2 inflight commands instead of 1 from hctx0 context +
> > > > From
> > > > hctx1 context, we will count 1 inflight command = Total is 3.
> > > > Even though we read after some delay, host_busy will be incorrect.
> > > > We expect host_busy = 2 but it will return 3.
> > > >
> > > > This patch fix my issue explained above for shared host-tag case.
> > > > I am confused reading the commit message. You may not have
> > > > intentionally fix the issue as I explained but indirectly it fixes
> > > > my issue. Am I
> > correct ?
> > > >
> > > > What was an issue reported by Luojiaxiang ? I am interested to
> > > > know if issue reported by Luojiaxiang had shared host tagset
enabled ?
> > >
> > > https://lore.kernel.org/linux-scsi/fe5cf6c4-ce5e-4a0f-f4ab-
> > > 5c10539492cb@huawei.com/
> >
> > I check this. It is same issue as what I am seeing on Broadcom
> > controller only if shared host tagset is enabled.
>
> But 67f3b2f822b7 ("blk-mq: avoid to iterate over stale request") isn't
only for
> shared tags.

I mean, shared bitmap in my whole discussion. Megaraid_sas driver use
shared bitmap, so it is exposed and It is confirmed from this discussion.
Do we still have exposure (if "blk-mq: avoid to iterate over stale
request" is not part of kernel)  to mpi3mr type driver which does not use
shared bitmap but has nr_hw_queues > 1. ?

Kashyap

>
>
>
> Thanks
> Ming

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]