| Message ID | 20200520170635.2094101-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] blk-mq: remove the bio argument to ->prepare_request | expand |
On 2020-05-20 10:06, Christoph Hellwig wrote: > @@ -321,8 +324,30 @@ static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, > busy_tag_iter_fn *fn, void *priv) > { > if (tags->nr_reserved_tags) > - bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true); > - bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false); > + bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true, > + false); > + bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false, false); > +} Adding /*reserved=*/ and /*iterate_all=*/ comments in front of the boolean arguments would make this code easier to read. > +/** > + * blk_mq_all_tag_iter - iterate over all requests in a tag map > + * @tags: Tag map to iterate over. > + * @fn: Pointer to the function that will be called for each > + * request. @fn will be called as follows: @fn(rq, @priv, > + * reserved) where rq is a pointer to a request. 'reserved' > + * indicates whether or not @rq is a reserved request. Return > + * true to continue iterating tags, false to stop. > + * @priv: Will be passed as second argument to @fn. > + * > + * It is the caller's responsibility to check rq's state in @fn. ^^^^^^ @fn? Not sure how the blk_mq_all_tag_iter() caller can check the request state ... > + */ > +void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, > + void *priv) > +{ > + if (tags->nr_reserved_tags) > + bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true, > + true); > + bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false, true); > } Same comment here: I think that adding /*reserved=*/ and /*iterate_all=*/ comments in front of the boolean arguments would make this code easier to read. Otherwise this patch looks good to me. Thanks, Bart.
On 5/20/20 7:06 PM, Christoph Hellwig wrote: > From: Ming Lei <ming.lei@redhat.com> > > Add one new function of blk_mq_all_tag_iter so that we can iterate every > allocated request from either io scheduler tags or driver tags, and this > way is more flexible since it allows the callers to do whatever they want > on allocated request. > > It will be used to implement draining allocated requests on specified > hctx in this patchset. > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/blk-mq-tag.c | 33 +++++++++++++++++++++++++++++---- > block/blk-mq-tag.h | 2 ++ > 2 files changed, 31 insertions(+), 4 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On Wed, May 20, 2020 at 01:24:02PM -0700, Bart Van Assche wrote: > Adding /*reserved=*/ and /*iterate_all=*/ comments in front of the > boolean arguments would make this code easier to read. Actually, flags values would be much more understandable, and allow to consolidate more code. I've switched the implementation over to that.
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index 586c9d6e904ab..c27c6dfc7d36e 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -257,6 +257,7 @@ struct bt_tags_iter_data { busy_tag_iter_fn *fn; void *data; bool reserved; + bool iterate_all; }; static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) @@ -274,7 +275,7 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) * test and set the bit before assining ->rqs[]. */ rq = tags->rqs[bitnr]; - if (rq && blk_mq_request_started(rq)) + if (rq && (iter_data->iterate_all || blk_mq_request_started(rq))) return iter_data->fn(rq, iter_data->data, reserved); return true; @@ -294,13 +295,15 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) * bitmap_tags member of struct blk_mq_tags. */ static void bt_tags_for_each(struct blk_mq_tags *tags, struct sbitmap_queue *bt, - busy_tag_iter_fn *fn, void *data, bool reserved) + busy_tag_iter_fn *fn, void *data, bool reserved, + bool iterate_all) { struct bt_tags_iter_data iter_data = { .tags = tags, .fn = fn, .data = data, .reserved = reserved, + .iterate_all = iterate_all, }; if (tags->rqs) @@ -321,8 +324,30 @@ static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, void *priv) { if (tags->nr_reserved_tags) - bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true); - bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false); + bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true, + false); + bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false, false); +} + +/** + * blk_mq_all_tag_iter - iterate over all requests in a tag map + * @tags: Tag map to iterate over. + * @fn: Pointer to the function that will be called for each + * request. @fn will be called as follows: @fn(rq, @priv, + * reserved) where rq is a pointer to a request. 'reserved' + * indicates whether or not @rq is a reserved request. Return + * true to continue iterating tags, false to stop. + * @priv: Will be passed as second argument to @fn. + * + * It is the caller's responsibility to check rq's state in @fn. + */ +void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, + void *priv) +{ + if (tags->nr_reserved_tags) + bt_tags_for_each(tags, &tags->breserved_tags, fn, priv, true, + true); + bt_tags_for_each(tags, &tags->bitmap_tags, fn, priv, false, true); } /** diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h index 2b8321efb6820..d19546e8246b7 100644 --- a/block/blk-mq-tag.h +++ b/block/blk-mq-tag.h @@ -34,6 +34,8 @@ extern int blk_mq_tag_update_depth(struct blk_mq_hw_ctx *hctx, extern void blk_mq_tag_wakeup_all(struct blk_mq_tags *tags, bool); void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn, void *priv); +void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, + void *priv); static inline struct sbq_wait_state *bt_wait_ptr(struct sbitmap_queue *bt, struct blk_mq_hw_ctx *hctx)