* [PATCH 0/3] cleanups/fix for blk_mq_get_tag()
@ 2020-11-30 1:36 Pavel Begunkov
2020-11-30 1:36 ` [PATCH 1/3] blk-mq: use right tag offset after wait Pavel Begunkov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-11-30 1:36 UTC (permalink / raw)
To: Jens Axboe, linux-block
[1/3] fixes using tag_offset of a different HW queue. May only fire
if it's allowed for queues to have different breserved_tags or a resize
happened while it was waiting (can be?). Two others are simple cleanups
catched the eye.
Pavel Begunkov (3):
blk-mq: use right tag offset after wait
blk-mq: deduplicate blk_mq_get_tag() bits
blk-mq: idiomatic use of WARN_ON_ONCE
block/blk-mq-tag.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] blk-mq: use right tag offset after wait 2020-11-30 1:36 [PATCH 0/3] cleanups/fix for blk_mq_get_tag() Pavel Begunkov @ 2020-11-30 1:36 ` Pavel Begunkov 2020-12-03 6:49 ` Ming Lei 2020-11-30 1:36 ` [PATCH 2/3] blk-mq: deduplicate blk_mq_get_tag() bits Pavel Begunkov 2020-11-30 1:36 ` [PATCH 3/3] blk-mq: idiomatic use of WARN_ON_ONCE Pavel Begunkov 2 siblings, 1 reply; 6+ messages in thread From: Pavel Begunkov @ 2020-11-30 1:36 UTC (permalink / raw) To: Jens Axboe, linux-block blk_mq_get_tag() selects tag_offset in the beginning and doesn't update it even though the tag set it may have changed hw queue during waiting. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/blk-mq-tag.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index 9c92053e704d..dbbf11edf9a6 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -101,10 +101,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) return BLK_MQ_NO_TAG; } bt = tags->breserved_tags; - tag_offset = 0; } else { bt = tags->bitmap_tags; - tag_offset = tags->nr_reserved_tags; } tag = __blk_mq_get_tag(data, bt); @@ -167,6 +165,11 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) sbitmap_finish_wait(bt, ws, &wait); found_tag: + if (data->flags & BLK_MQ_REQ_RESERVED) + tag_offset = 0; + else + tag_offset = tags->nr_reserved_tags; + /* * Give up this allocation if the hctx is inactive. The caller will * retry on an active hctx. -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] blk-mq: use right tag offset after wait 2020-11-30 1:36 ` [PATCH 1/3] blk-mq: use right tag offset after wait Pavel Begunkov @ 2020-12-03 6:49 ` Ming Lei 2020-12-03 12:06 ` Pavel Begunkov 0 siblings, 1 reply; 6+ messages in thread From: Ming Lei @ 2020-12-03 6:49 UTC (permalink / raw) To: Pavel Begunkov; +Cc: Jens Axboe, linux-block On Mon, Nov 30, 2020 at 01:36:25AM +0000, Pavel Begunkov wrote: > blk_mq_get_tag() selects tag_offset in the beginning and doesn't update > it even though the tag set it may have changed hw queue during waiting. > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > block/blk-mq-tag.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c > index 9c92053e704d..dbbf11edf9a6 100644 > --- a/block/blk-mq-tag.c > +++ b/block/blk-mq-tag.c > @@ -101,10 +101,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) > return BLK_MQ_NO_TAG; > } > bt = tags->breserved_tags; > - tag_offset = 0; > } else { > bt = tags->bitmap_tags; > - tag_offset = tags->nr_reserved_tags; > } > > tag = __blk_mq_get_tag(data, bt); > @@ -167,6 +165,11 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) > sbitmap_finish_wait(bt, ws, &wait); > > found_tag: > + if (data->flags & BLK_MQ_REQ_RESERVED) > + tag_offset = 0; > + else > + tag_offset = tags->nr_reserved_tags; > + So far, the tag offset is tag-set wide, and 'tag_offset' is same for all tags, looks no need to add the extra check. thanks, Ming ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] blk-mq: use right tag offset after wait 2020-12-03 6:49 ` Ming Lei @ 2020-12-03 12:06 ` Pavel Begunkov 0 siblings, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-12-03 12:06 UTC (permalink / raw) To: Ming Lei; +Cc: Jens Axboe, linux-block On 03/12/2020 06:49, Ming Lei wrote: > On Mon, Nov 30, 2020 at 01:36:25AM +0000, Pavel Begunkov wrote: >> blk_mq_get_tag() selects tag_offset in the beginning and doesn't update >> it even though the tag set it may have changed hw queue during waiting. >> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> --- >> block/blk-mq-tag.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index 9c92053e704d..dbbf11edf9a6 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -101,10 +101,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) >> return BLK_MQ_NO_TAG; >> } >> bt = tags->breserved_tags; >> - tag_offset = 0; >> } else { >> bt = tags->bitmap_tags; >> - tag_offset = tags->nr_reserved_tags; >> } >> >> tag = __blk_mq_get_tag(data, bt); >> @@ -167,6 +165,11 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) >> sbitmap_finish_wait(bt, ws, &wait); >> >> found_tag: >> + if (data->flags & BLK_MQ_REQ_RESERVED) >> + tag_offset = 0; >> + else >> + tag_offset = tags->nr_reserved_tags; >> + > > So far, the tag offset is tag-set wide, and 'tag_offset' is same for all tags, > looks no need to add the extra check. Thanks for clearing that. I still think that's a good thing to do as more apparent to me and shouldn't be slower -- it unconditionally stores offset on-stack, even for no-wait path (gcc 9.2). With it's optimised more like if (!(data->flags & BLK_MQ_REQ_RESERVED)) tag += tags->nr_reserved_tags; I like assembly a bit more, but not like that matters much. -- Pavel Begunkov ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] blk-mq: deduplicate blk_mq_get_tag() bits 2020-11-30 1:36 [PATCH 0/3] cleanups/fix for blk_mq_get_tag() Pavel Begunkov 2020-11-30 1:36 ` [PATCH 1/3] blk-mq: use right tag offset after wait Pavel Begunkov @ 2020-11-30 1:36 ` Pavel Begunkov 2020-11-30 1:36 ` [PATCH 3/3] blk-mq: idiomatic use of WARN_ON_ONCE Pavel Begunkov 2 siblings, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-11-30 1:36 UTC (permalink / raw) To: Jens Axboe, linux-block Deduplicate bt_wait_ptr() call in blk_mq_get_tag(), condition-less while and absence of loop controls in-between former and current call location makes the change functionally identical. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/blk-mq-tag.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index dbbf11edf9a6..98f6ea219bb4 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -112,10 +112,11 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) if (data->flags & BLK_MQ_REQ_NOWAIT) return BLK_MQ_NO_TAG; - ws = bt_wait_ptr(bt, data->hctx); do { struct sbitmap_queue *bt_prev; + ws = bt_wait_ptr(bt, data->hctx); + /* * We're out of tags on this hardware queue, kick any * pending IO submits before going to sleep waiting for @@ -158,8 +159,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) */ if (bt != bt_prev) sbitmap_queue_wake_up(bt_prev); - - ws = bt_wait_ptr(bt, data->hctx); } while (1); sbitmap_finish_wait(bt, ws, &wait); -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] blk-mq: idiomatic use of WARN_ON_ONCE 2020-11-30 1:36 [PATCH 0/3] cleanups/fix for blk_mq_get_tag() Pavel Begunkov 2020-11-30 1:36 ` [PATCH 1/3] blk-mq: use right tag offset after wait Pavel Begunkov 2020-11-30 1:36 ` [PATCH 2/3] blk-mq: deduplicate blk_mq_get_tag() bits Pavel Begunkov @ 2020-11-30 1:36 ` Pavel Begunkov 2 siblings, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2020-11-30 1:36 UTC (permalink / raw) To: Jens Axboe, linux-block Let WARN_ON_ONCE() to be more clear by passing the warn condition directly, so it'll be printed. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/blk-mq-tag.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index 98f6ea219bb4..f2743a9159ca 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -96,10 +96,8 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) int tag; if (data->flags & BLK_MQ_REQ_RESERVED) { - if (unlikely(!tags->nr_reserved_tags)) { - WARN_ON_ONCE(1); + if (WARN_ON_ONCE(!tags->nr_reserved_tags)) return BLK_MQ_NO_TAG; - } bt = tags->breserved_tags; } else { bt = tags->bitmap_tags; -- 2.24.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-03 12:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-30 1:36 [PATCH 0/3] cleanups/fix for blk_mq_get_tag() Pavel Begunkov 2020-11-30 1:36 ` [PATCH 1/3] blk-mq: use right tag offset after wait Pavel Begunkov 2020-12-03 6:49 ` Ming Lei 2020-12-03 12:06 ` Pavel Begunkov 2020-11-30 1:36 ` [PATCH 2/3] blk-mq: deduplicate blk_mq_get_tag() bits Pavel Begunkov 2020-11-30 1:36 ` [PATCH 3/3] blk-mq: idiomatic use of WARN_ON_ONCE Pavel Begunkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).