Add queue_is_busy helper

PATCH In review Jens Axboe 3 participants 14 messages Nov 8, 2018 – Nov 9, 2018 Block layer

Share & tools

Jens Axboe

author maintainer

2018-11-08 16:06 UTC #0 lore raw

v2 cover Add queue_is_busy helper

DM currently uses atomic inc/dec to maintain a busy count of IO on a given device. For the dm-mq path, we can replace this with helper that just checks the state of the tags on the device.

First patch is a prep patch that allows the iteration helpers to return true/false, like we support internally in sbitmap. For a busy check we don't care about how many requests are busy, just if some are or not. Hence we can stop iterating tags as soon as we find one that is allocated.

Changes since v1:

- Remember to check if the queue matches, otherwise we could be returning false positive for shared tag sets.

 block/blk-mq-debugfs.c |  4 +++-
 block/blk-mq-tag.c     |  4 ++--
 block/blk-mq.c         | 42 +++++++++++++++++++++++++++++++++++++-----
 include/linux/blk-mq.h |  6 ++++--
 4 files changed, 46 insertions(+), 10 deletions(-)

Signature · 1 line

Jens Axboe

Jens Axboe

author maintainer

2018-11-08 16:06 UTC #1 lore raw

PATCH 1/1 blk-mq: provide a helper to check if a queue is busy

Returns true if the queue currently has requests pending, false if not.

DM can use this to replace the atomic_inc/dec they do per device to see if a device is busy.

Cc Mike Snitzer <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 block/blk-mq.c         | 22 ++++++++++++++++++++++
 include/linux/blk-mq.h |  2 ++
 2 files changed, 24 insertions(+)

block/blk-mq.c +22 -0

index 4a622c832b31..848adcc51c43 100644
@@ -790,6 +790,28 @@ struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)
 }
 EXPORT_SYMBOL(blk_mq_tag_to_rq);
 
+static bool blk_mq_check_busy(struct blk_mq_hw_ctx *hctx, struct request *rq,
+			      void *priv, bool reserved)
+{
+	bool *busy = (bool *) priv;
+
+	/*
+	 * If we find a request, we know the queue is busy. Return false
+	 * to stop the iteration.
+	 */
+	*busy = true;
+	return false;
+}
+
+bool blk_mq_queue_busy(struct request_queue *q)
+{
+	bool busy = false;
+
+	blk_mq_queue_tag_busy_iter(q, blk_mq_check_busy, &busy);
+	return busy;
+}
+EXPORT_SYMBOL_GPL(blk_mq_queue_busy);
+
 static void blk_mq_rq_timed_out(struct request *req, bool reserved)
 {
 	req->rq_flags |= RQF_TIMED_OUT;

include/linux/blk-mq.h +2 -0

index ff497dfcbbf9..929e8abc5535 100644
@@ -250,6 +250,8 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule);
 void blk_mq_free_request(struct request *rq);
 bool blk_mq_can_queue(struct blk_mq_hw_ctx *);
 
+bool blk_mq_queue_busy(struct request_queue *q);
+
 enum {
 	/* return when out of requests */
 	BLK_MQ_REQ_NOWAIT	= (__force blk_mq_req_flags_t)(1 << 0),

Signature · 1 line

2.17.1

Jens Axboe

author maintainer

2018-11-08 16:06 UTC #2 lore raw

PATCH v2 1/2 blk-mq-tag: change busy_iter_fn to return whether to continue or not

In reply to Jens Axboe

We have this functionality in sbitmap, but we don't export it in blk-mq for users of the tags busy iteration. This can be useful for stopping the iteration, if the caller doesn't need to find more requests.

Reviewed-by Mike Snitzer <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 block/blk-mq-debugfs.c |  4 +++-
 block/blk-mq-tag.c     |  4 ++--
 block/blk-mq.c         | 16 +++++++++++-----
 include/linux/blk-mq.h |  4 ++--
 4 files changed, 18 insertions(+), 10 deletions(-)

block/blk-mq-debugfs.c +3 -1

index cde19be36135..4f61632af6f4 100644
@@ -424,13 +424,15 @@ struct show_busy_params {
  * Note: the state of a request may change while this function is in progress,
  * e.g. due to a concurrent blk_mq_finish_request() call.
  */
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
+static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
 {
 	const struct show_busy_params *params = data;
 
 	if (rq->mq_hctx == params->hctx)
 		__blk_mq_debugfs_rq_show(params->m,
 					 list_entry_rq(&rq->queuelist));
+
+	return true;
 }
 
 static int hctx_busy_show(void *data, struct seq_file *m)

block/blk-mq-tag.c +2 -2

index fb836d818b80..097e9a67d5f5 100644
@@ -236,7 +236,7 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 	 * test and set the bit before assigning ->rqs[].
 	 */
 	if (rq && rq->q == hctx->queue)
-		iter_data->fn(hctx, rq, iter_data->data, reserved);
+		return iter_data->fn(hctx, rq, iter_data->data, reserved);
 	return true;
 }

@@ -289,7 +289,7 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 	 */
 	rq = tags->rqs[bitnr];
 	if (rq && blk_mq_request_started(rq))
-		iter_data->fn(rq, iter_data->data, reserved);
+		return iter_data->fn(rq, iter_data->data, reserved);
 
 	return true;
 }

block/blk-mq.c +11 -5

index 45c92b8d4795..4a622c832b31 100644
@@ -94,7 +94,7 @@ struct mq_inflight {
 	unsigned int *inflight;
 };
 
-static void blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
+static bool blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
 				  struct request *rq, void *priv,
 				  bool reserved)
 {

@@ -109,6 +109,8 @@ static void blk_mq_check_inflight(struct blk_mq_hw_ctx *hctx,
 		mi->inflight[0]++;
 	if (mi->part->partno)
 		mi->inflight[1]++;
+
+	return true;
 }
 
 void blk_mq_in_flight(struct request_queue *q, struct hd_struct *part,

@@ -120,7 +122,7 @@ void blk_mq_in_flight(struct request_queue *q, struct hd_struct *part,
 	blk_mq_queue_tag_busy_iter(q, blk_mq_check_inflight, &mi);
 }
 
-static void blk_mq_check_inflight_rw(struct blk_mq_hw_ctx *hctx,
+static bool blk_mq_check_inflight_rw(struct blk_mq_hw_ctx *hctx,
 				     struct request *rq, void *priv,
 				     bool reserved)
 {

@@ -128,6 +130,8 @@ static void blk_mq_check_inflight_rw(struct blk_mq_hw_ctx *hctx,
 
 	if (rq->part == mi->part)
 		mi->inflight[rq_data_dir(rq)]++;
+
+	return true;
 }
 
 void blk_mq_in_flight_rw(struct request_queue *q, struct hd_struct *part,

@@ -821,7 +825,7 @@ static bool blk_mq_req_expired(struct request *rq, unsigned long *next)
 	return false;
 }
 
-static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
+static bool blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 		struct request *rq, void *priv, bool reserved)
 {
 	unsigned long *next = priv;

@@ -831,7 +835,7 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 	 * so we're not unnecessarilly synchronizing across CPUs.
 	 */
 	if (!blk_mq_req_expired(rq, next))
-		return;
+		return true;
 
 	/*
 	 * We have reason to believe the request may be expired. Take a

@@ -843,7 +847,7 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 	 * timeout handler to posting a natural completion.
 	 */
 	if (!refcount_inc_not_zero(&rq->ref))
-		return;
+		return true;
 
 	/*
 	 * The request is now locked and cannot be reallocated underneath the

@@ -855,6 +859,8 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx,
 		blk_mq_rq_timed_out(rq, reserved);
 	if (refcount_dec_and_test(&rq->ref))
 		__blk_mq_free_request(rq);
+
+	return true;
 }
 
 static void blk_mq_timeout_work(struct work_struct *work)

include/linux/blk-mq.h +2 -2

index 9f5e93f40857..ff497dfcbbf9 100644
@@ -129,9 +129,9 @@ typedef int (init_request_fn)(struct blk_mq_tag_set *set, struct request *,
 typedef void (exit_request_fn)(struct blk_mq_tag_set *set, struct request *,
 		unsigned int);
 
-typedef void (busy_iter_fn)(struct blk_mq_hw_ctx *, struct request *, void *,
+typedef bool (busy_iter_fn)(struct blk_mq_hw_ctx *, struct request *, void *,
 		bool);
-typedef void (busy_tag_iter_fn)(struct request *, void *, bool);
+typedef bool (busy_tag_iter_fn)(struct request *, void *, bool);
 typedef int (poll_fn)(struct blk_mq_hw_ctx *, unsigned int);
 typedef int (map_queues_fn)(struct blk_mq_tag_set *set);
 typedef bool (busy_fn)(struct request_queue *);

Signature · 1 line

2.17.1

Bart Van Assche

2018-11-08 16:28 UTC #3 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 8 lines · Jens Axboe

--- a/block/blk-mq-debugfs.c
+-+-+- b/block/blk-mq-debugfs.c
@@-424,13 +-424,15 @@struct show_busy_params {>  *Note: the state of a request may change while this function is in progress,
*e.g. due to a concurrent blk_mq_finish_request() call.
*/
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved) +-static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved) {

Please update the kdoc header above hctx_show_busy_rq() such that it reflects the new behavior. I'm referring to the "will be called for each request"part. Otherwise this patch looks fine to me.

Bart.

Jens Axboe

author maintainer

2018-11-08 16:31 UTC #4 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 14 lines · Bart Van Assche

block/blk-mq-debugfs.c struct show_busy_params { · line 428
Quoted 9 lines · Jens Axboe
block/blk-mq-debugfs.c +1 -1
@@ -424,13 +424,15 @@ struct show_busy_params {
  * Note: the state of a request may change while this function is in progress,
  * e.g. due to a concurrent blk_mq_finish_request() call.
  */
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
+static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
 {
Please update the kdoc header above hctx_show_busy_rq() such that it reflects the new behavior. I'm referring to the "will be called for each request" part. Otherwise this patch looks fine to me.

Took a look at the comment, and what do you want changed? There's no change in behavior for hctx_show_busy_rq(), it loops all requests just like before. We just return true to ensure we continue iterating.

Signature · 1 line

Jens Axboe

Bart Van Assche

2018-11-09 03:11 UTC #5 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 14 lines · Jens Axboe

Quoted 12 lines · Bart Van Assche
Quoted 8 lines · Jens Axboe
--- a/block/blk-mq-debugfs.c
+-+-+- b/block/blk-mq-debugfs.c
@@-424,13 +-424,15 @@struct show_busy_params {>>>  *Note: the state of a request may change while this function is in progress,
*e.g. due to a concurrent blk_mq_finish_request() call.
*/
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved) +-static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved) {>>
Please update the kdoc header above hctx_show_busy_rq() such that it reflects the new behavior. I'm referring to the "will be called for each request"part. Otherwise this patch looks fine to me.

Quoted 3 lines

Took a look at the comment, and what do you want changed? There's no change in behavior for hctx_show_busy_rq(), it loops all requests just like before. We just return true to ensure we continue iterating.

Oops, I added my reply below the wrong function. I wanted to refer to the following comment above blk_mq_queue_tag_busy_iter():

*@fn: Pointer to the function that will be called for each request

Additionally, how about similar comments above bt_for_each(), bt_tags_for_each() blk_mq_all_tag_busy_iter() and blk_mq_tagset_busy_iter()? I think this patch affects all these functions.

Thanks,

Bart.

Jens Axboe

author maintainer

2018-11-08 17:47 UTC #6 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 21 lines · Bart Van Assche

Quoted 16 lines · Jens Axboe
Quoted 14 lines · Bart Van Assche
block/blk-mq-debugfs.c struct show_busy_params { · line 428
Quoted 9 lines · Jens Axboe
block/blk-mq-debugfs.c +1 -1
@@ -424,13 +424,15 @@ struct show_busy_params {
  * Note: the state of a request may change while this function is in progress,
  * e.g. due to a concurrent blk_mq_finish_request() call.
  */
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
+static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
 {
Please update the kdoc header above hctx_show_busy_rq() such that it reflects the new behavior. I'm referring to the "will be called for each request" part. Otherwise this patch looks fine to me.
Quoted 3 lines
Took a look at the comment, and what do you want changed? There's no change in behavior for hctx_show_busy_rq(), it loops all requests just like before. We just return true to ensure we continue iterating.

Quoted 8 lines

Oops, I added my reply below the wrong function. I wanted to refer to the following comment above blk_mq_queue_tag_busy_iter():
* @fn: Pointer to the function that will be called for each request
Additionally, how about similar comments above bt_for_each(), bt_tags_for_each() blk_mq_all_tag_busy_iter() and blk_mq_tagset_busy_iter()? I think this patch affects all these functions.

Fair enough, I'll add a documentation patch. Didn't consider it a big deal since this is how it's always worked internally, but we haven't exposed this break/continue functionality outside the sbitmap core until now.

Signature · 1 line

Jens Axboe

Jens Axboe

author maintainer

2018-11-08 17:50 UTC #7 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 27 lines · Jens Axboe

Quoted 26 lines · Bart Van Assche
Quoted 16 lines · Jens Axboe
Quoted 14 lines · Bart Van Assche
block/blk-mq-debugfs.c struct show_busy_params { · line 428
Quoted 9 lines · Jens Axboe
block/blk-mq-debugfs.c +1 -1
@@ -424,13 +424,15 @@ struct show_busy_params {
  * Note: the state of a request may change while this function is in progress,
  * e.g. due to a concurrent blk_mq_finish_request() call.
  */
-static void hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
+static bool hctx_show_busy_rq(struct request *rq, void *data, bool reserved)
 {
Please update the kdoc header above hctx_show_busy_rq() such that it reflects the new behavior. I'm referring to the "will be called for each request" part. Otherwise this patch looks fine to me.
Quoted 3 lines
Took a look at the comment, and what do you want changed? There's no change in behavior for hctx_show_busy_rq(), it loops all requests just like before. We just return true to ensure we continue iterating.
Oops, I added my reply below the wrong function. I wanted to refer to the following comment above blk_mq_queue_tag_busy_iter():
* @fn: Pointer to the function that will be called for each request

Quoted 7 lines

Quoted 3 lines
Additionally, how about similar comments above bt_for_each(), bt_tags_for_each() blk_mq_all_tag_busy_iter() and blk_mq_tagset_busy_iter()? I think this patch affects all these functions.
Fair enough, I'll add a documentation patch. Didn't consider it a big deal since this is how it's always worked internally, but we haven't exposed this break/continue functionality outside the sbitmap core until now.

How about this incremental?

block/blk-mq-tag.c +8 -4

index 097e9a67d5f5..87bc5df72d48 100644
@@ -248,7 +248,8 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
  * @fn:		Pointer to the function that will be called for each request
  *		associated with @hctx that has been assigned a driver tag.
  *		@fn will be called as follows: @fn(@hctx, rq, @data, @reserved)
- *		where rq is a pointer to a request.
+ *		where rq is a pointer to a request. Return true to continue
+ *		iterating tags, false to stop.
  * @data:	Will be passed as third argument to @fn.
  * @reserved:	Indicates whether @bt is the breserved_tags member or the
  *		bitmap_tags member of struct blk_mq_tags.

@@ -301,7 +302,8 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
  *		or the bitmap_tags member of struct blk_mq_tags.
  * @fn:		Pointer to the function that will be called for each started
  *		request. @fn will be called as follows: @fn(rq, @data,
- *		@reserved) where rq is a pointer to a request.
+ *		@reserved) where rq is a pointer to a request. Return true
+ *		to continue iterating tags, false to stop.
  * @data:	Will be passed as second argument to @fn.
  * @reserved:	Indicates whether @bt is the breserved_tags member or the
  *		bitmap_tags member of struct blk_mq_tags.

@@ -326,7 +328,8 @@ static void bt_tags_for_each(struct blk_mq_tags *tags, struct sbitmap_queue *bt,
  * @fn:		Pointer to the function that will be called for each started
  *		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.
+ *		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.
  */
 static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags,

@@ -343,7 +346,8 @@ static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags,
  * @fn:		Pointer to the function that will be called for each started
  *		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.
+ *		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.
  */
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,

Signature · 1 line

Jens Axboe

Bart Van Assche

2018-11-08 18:04 UTC #8 lore raw

Re: [PATCH 1/2] blk-mq-tag: change busy_iter_fn to return whether to continue or not

Quoted 41 lines · Jens Axboe

How about this incremental?
block/blk-mq-tag.c +1 -1
index 097e9a67d5f5..87bc5df72d48 100644
--- a/block/blk-mq-tag.c
+-+-+- b/block/blk-mq-tag.c
@@-248,7 +-248,8 @@static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
*@fn: Pointer to the function that will be called for each request
* associated with @hctx that has been assigned a driver tag.
* @fn will be called as follows: @fn(@hctx, rq, @data, @reserved)
- * where rq is a pointer to a request.
+- *	where rq is a pointer to a request. Return true to continue
+- *	iterating tags, false to stop.
*@data: Will be passed as third argument to @fn.
*@reserved: Indicates whether @bt is the breserved_tags member or the
* bitmap_tags member of struct blk_mq_tags.
@@-301,7 +-302,8 @@static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
* or the bitmap_tags member of struct blk_mq_tags.
*@fn: Pointer to the function that will be called for each started
* request. @fn will be called as follows: @fn(rq, @data,
- * @reserved) where rq is a pointer to a request.
+- *	@reserved) where rq is a pointer to a request. Return true
+- *	to continue iterating tags, false to stop.
*@data: Will be passed as second argument to @fn.
*@reserved: Indicates whether @bt is the breserved_tags member or the
* bitmap_tags member of struct blk_mq_tags.
@@-326,7 +-328,8 @@static void bt_tags_for_each(struct blk_mq_tags *tags, struct sbitmap_queue *bt,
*@fn: Pointer to the function that will be called for each started
* 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.
+- *	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.
*/
static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, @@-343,7 +-346,8 @@static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags,
*@fn: Pointer to the function that will be called for each started
* 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.

Quoted 5 lines

+- *	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.
*/
void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,

This looks fine to me. Thanks!
Bart.

Jens Axboe

author maintainer

2018-11-08 16:06 UTC #9 lore raw

PATCH v2 2/2 blk-mq: provide a helper to check if a queue is busy

In reply to Jens Axboe

Returns true if the queue currently has requests pending, false if not.

DM can use this to replace the atomic_inc/dec they do per device to see if a device is busy.

Reviewed-by Mike Snitzer <[email protected]>

Signed-off-by Jens Axboe <[email protected]>

---
 block/blk-mq.c         | 26 ++++++++++++++++++++++++++
 include/linux/blk-mq.h |  2 ++
 2 files changed, 28 insertions(+)

block/blk-mq.c +26 -0

index 4a622c832b31..65243c40ac7c 100644
@@ -790,6 +790,32 @@ struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag)
 }
 EXPORT_SYMBOL(blk_mq_tag_to_rq);
 
+static bool blk_mq_check_busy(struct blk_mq_hw_ctx *hctx, struct request *rq,
+			      void *priv, bool reserved)
+{
+	/*
+	 * If we find a request, we know the queue is busy. Return false
+	 * to stop the iteration.
+	 */
+	if (rq->q == hctx->queue) {
+		bool *busy = (bool *) priv;
+
+		*busy = true;
+		return false;
+	}
+
+	return false;
+}
+
+bool blk_mq_queue_busy(struct request_queue *q)
+{
+	bool busy = false;
+
+	blk_mq_queue_tag_busy_iter(q, blk_mq_check_busy, &busy);
+	return busy;
+}
+EXPORT_SYMBOL_GPL(blk_mq_queue_busy);
+
 static void blk_mq_rq_timed_out(struct request *req, bool reserved)
 {
 	req->rq_flags |= RQF_TIMED_OUT;

include/linux/blk-mq.h +2 -0

index ff497dfcbbf9..929e8abc5535 100644
@@ -250,6 +250,8 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule);
 void blk_mq_free_request(struct request *rq);
 bool blk_mq_can_queue(struct blk_mq_hw_ctx *);
 
+bool blk_mq_queue_busy(struct request_queue *q);
+
 enum {
 	/* return when out of requests */
 	BLK_MQ_REQ_NOWAIT	= (__force blk_mq_req_flags_t)(1 << 0),

Signature · 1 line

2.17.1

Bart Van Assche

Reviewed-by on v2 2/2

2018-11-08 16:30 UTC #10 lore raw

Re: [PATCH 2/2] blk-mq: provide a helper to check if a queue is busy

Quoted 6 lines · Jens Axboe

+-static bool blk_mq_check_busy(struct blk_mq_hw_ctx *hctx, struct request *rq,
+-			      void *priv, bool reserved)
+-{>+-	/*>+-	 *If we find a request, we know the queue is busy. Return false
+-	 *to stop the iteration.
+-	 */
+-	if (rq->q ==hctx->queue) {>+-		bool *busy =(bool * priv;

I think the "bool *"cast can be left out. Anyway:

Reviewed-by Bart Van Assche <[email protected]>

Jens Axboe

author maintainer

2018-11-08 16:32 UTC #11 lore raw

Re: [PATCH 2/2] blk-mq: provide a helper to check if a queue is busy

Quoted 12 lines · Bart Van Assche

block/blk-mq.c struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag) · line 801
Quoted 9 lines · Jens Axboe
block/blk-mq.c +9 -0
+static bool blk_mq_check_busy(struct blk_mq_hw_ctx *hctx, struct request *rq,
+			      void *priv, bool reserved)
+{
+	/*
+	 * If we find a request, we know the queue is busy. Return false
+	 * to stop the iteration.
+	 */
+	if (rq->q == hctx->queue) {
+		bool *busy = (bool *) priv;
I think the "(bool *)" cast can be left out. Anyway:

It can, I'll drop it.

Quoted 1 line

Reviewed-by Bart Van Assche <[email protected]>

Thanks

Signature · 1 line

Jens Axboe

jianchao.wang

2018-11-09 01:53 UTC #12 lore raw

In reply to Jens Axboe

Hi Jens

Quoted 9 lines · Jens Axboe

DM currently uses atomic inc/dec to maintain a busy count of IO on a given device. For the dm-mq path, we can replace this with helper that just checks the state of the tags on the device.
First patch is a prep patch that allows the iteration helpers to return true/false, like we support internally in sbitmap. For a busy check we don't care about how many requests are busy, just if some are or not. Hence we can stop iterating tags as soon as we find one that is allocated.

If we don't care about how many requests are busy, why not check sb->map[idex].word directly ? It could be more efficient.

Thanks
Jianchao

jianchao.wang

2018-11-09 02:08 UTC #13 lore raw

Quoted 7 lines · jianchao.wang

Hi Jens
Quoted 4 lines · Jens Axboe
DM currently uses atomic inc/dec to maintain a busy count of IO on a given device. For the dm-mq path, we can replace this with helper that just checks the state of the tags on the device.

Quoted 9 lines

Quoted 5 lines
First patch is a prep patch that allows the iteration helpers to return true/false, like we support internally in sbitmap. For a busy check we don't care about how many requests are busy, just if some are or not. Hence we can stop iterating tags as soon as we find one that is allocated.
If we don't care about how many requests are busy, why not check sb->map[idex].word directly ? It could be more efficient.

Oh, we need to check rq->q for the tag shared case.

Quoted 3 lines

Thanks
Jianchao