On Thu, Nov 20, 2025 at 4:37 AM Tao Liu <[email protected]> wrote:
> Hi lianbo,
>
> On Wed, Nov 19, 2025 at 10:31 PM lijiang <[email protected]> wrote:
> >
> > On Wed, Nov 19, 2025 at 12:51 PM <
> [email protected]> wrote:
> >>
> >> Date: Wed, 19 Nov 2025 15:36:29 +1300
> >> From: Tao Liu <[email protected]>
> >> Subject: [Crash-utility] [PATCH] Add blk_mq_tag_set shared_tags check
> >> To: [email protected]
> >> Cc: Tao Liu <[email protected]>
> >> Message-ID: <[email protected]>
> >> Content-Type: text/plain; charset="US-ASCII"; x-default=true
> >>
> >> A regression of cmd dev -d/-D noticed on some vmcores as:
> >>
> >> dev: invalid structure member offset: blk_mq_tag_set_shared_tags
> >> FILE: dev.c LINE: 4468 FUNCTION: blk_mq_queue_tag_busy_iter()
> >>
> >> MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC
> SYNC
> >> [crash] error trace: 91aa85 => 91a552 => 94d6ba => 94d647
> >>
> >> 94d647: OFFSET_verify.part.0+55
> >> 94d6ba: OFFSET_verify+42
> >> 91a552: get_diskio_1+2194
> >> 91aa85: display_all_diskio+1253
> >>
> >> The root cause is, though upstream kernel have defined
> BLK_MQ_F_TAG_HCTX_SHARED
> >> as (1 << 3), the value might be set different, e.g. [1]. So a single
> >
> >
> > We should be able to get the value of BLK_MQ_F_TAG_HCTX_SHARED via the
> dump_enumerator_list(), and then test it in the following function:
>
> Please see [1], I removed (1<<3) and resolved it at runtime.
> dump_enumerator_list() is not appropriate, but I used
> arg_to_datatype().
>
Can you try the enumerator_value()? Looks more appropriate. E.g:
enumerator_value("BLK_MQ_F_TAG_HCTX_SHARED", &value);
Thanks
Lianbo
In dump_enumerator_list("BLK_MQ_F_TAG_HCTX_SHARED"),
>
> if (!arg_to_datatype(e, dm, RETURN_ON_ERROR) ||
> (dm->size < 0) || (dm->type != ENUM) || dm->tagname)
>
> dm->tagname is not NULL and will make dump_enumerator_list return false:
>
> (gdb) p dm
> $1 = {name = 0xe4911b "BLK_MQ_F_TAG_HCTX_SHARED", member = 0x0, type =
> 8192, size = 0, member_offset = 0, member_size = 0,
> member_typecode = 0, flags = 0, tagname = 0xd41dfd "(unknown)",
> value = 8, vaddr = 0}
>
> [1]:
> https://www.mail-archive.com/[email protected]/msg01841.html
>
> >
> > static bool blk_mq_is_shared_tags(unsigned int flags)
> > {
> > return flags & BLK_MQ_F_TAG_HCTX_SHARED;
> > }
> >
> > That can follow the changes once the upstream kernel modifies the value.
> >
> >> blk_mq_is_shared_tags() check is not sufficient. This patch will add
> shared_tagsblk_mq_is_shared_tags() check is not sufficient. This patch will
> add shared_tags
> >> member check within blk_mq_tag_set struct as an enhancment.
> >
> >
> > Anyway, this change looks good, so: Ack
> >
> > Lianbo
> >
> >> [1]:
> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-8/-/blob/c8s/include/linux/blk-mq.h?ref_type=heads#L321
> >>
> >> Fixes: 62486400d35b ("Add blk_mq shared tags support for dev -d/-D")
> >> Signed-off-by: Tao Liu <[email protected]>
> >> ---
> >> dev.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/dev.c b/dev.c
> >> index 8391d71..27318e8 100644
> >> --- a/dev.c
> >> +++ b/dev.c
> >> @@ -4464,7 +4464,7 @@ static void blk_mq_queue_tag_busy_iter(ulong q,
> ulong *hctx, uint cnt,
> >> readmem(addr + OFFSET(blk_mq_tag_set_flags), KVADDR,
> >> &flags, sizeof(uint), "blk_mq_tag_set.flags",
> RETURN_ON_ERROR);
> >>
> >> - if (blk_mq_is_shared_tags(flags)) {
> >> + if (blk_mq_is_shared_tags(flags) &&
> VALID_MEMBER(blk_mq_tag_set_shared_tags)) {
> >> addr = addr + OFFSET(blk_mq_tag_set_shared_tags);
> >> queue_for_each_hw_ctx(q, addr, bitmap_tags_is_ptr, dio);
> >> return;
> >> --
> >> 2.47.0
>
>
--
Crash-utility mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
