diff mbox series

[1/4] block: provide helpers for rq_list manipulation

Message ID 20211013164937.985367-2-axboe@kernel.dk (mailing list archive)
State New, archived
Headers show
Series Various block optimizations | expand

Commit Message

Jens Axboe Oct. 13, 2021, 4:49 p.m. UTC
Instead of open-coding the list additions, traversal, and removal,
provide a basic set of helpers.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-mq.c         | 21 +++++----------------
 include/linux/blk-mq.h | 25 +++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 16 deletions(-)

Comments

Christoph Hellwig Oct. 13, 2021, 5:11 p.m. UTC | #1
On Wed, Oct 13, 2021 at 10:49:34AM -0600, Jens Axboe wrote:
> Instead of open-coding the list additions, traversal, and removal,
> provide a basic set of helpers.
> 
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/blk-mq.c         | 21 +++++----------------
>  include/linux/blk-mq.h | 25 +++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 6dfd3aaa6073..46a91e5fabc5 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -426,10 +426,10 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
>  			tag = tag_offset + i;
>  			tags &= ~(1UL << i);
>  			rq = blk_mq_rq_ctx_init(data, tag, alloc_time_ns);
> -			rq->rq_next = *data->cached_rq;
> -			*data->cached_rq = rq;
> +			rq_list_add_tail(data->cached_rq, rq);
>  		}

This doesn't seem to match the code in the current for-5.6/block branch.

>  		data->nr_tags -= nr;
> +		return rq_list_pop(data->cached_rq);
>  	} else {

But either way no need for an else after a return.
Jens Axboe Oct. 13, 2021, 5:47 p.m. UTC | #2
On 10/13/21 11:11 AM, Christoph Hellwig wrote:
> On Wed, Oct 13, 2021 at 10:49:34AM -0600, Jens Axboe wrote:
>> Instead of open-coding the list additions, traversal, and removal,
>> provide a basic set of helpers.
>>
>> Suggested-by: Christoph Hellwig <hch@infradead.org>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  block/blk-mq.c         | 21 +++++----------------
>>  include/linux/blk-mq.h | 25 +++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 6dfd3aaa6073..46a91e5fabc5 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -426,10 +426,10 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
>>  			tag = tag_offset + i;
>>  			tags &= ~(1UL << i);
>>  			rq = blk_mq_rq_ctx_init(data, tag, alloc_time_ns);
>> -			rq->rq_next = *data->cached_rq;
>> -			*data->cached_rq = rq;
>> +			rq_list_add_tail(data->cached_rq, rq);
>>  		}
> 
> This doesn't seem to match the code in the current for-5.6/block branch.

It's after the 2 patch improvement series.

>>  		data->nr_tags -= nr;
>> +		return rq_list_pop(data->cached_rq);
>>  	} else {
> 
> But either way no need for an else after a return.

We can kill that, but it's really independent and unrelated to this patch.
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6dfd3aaa6073..46a91e5fabc5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -426,10 +426,10 @@  static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 			tag = tag_offset + i;
 			tags &= ~(1UL << i);
 			rq = blk_mq_rq_ctx_init(data, tag, alloc_time_ns);
-			rq->rq_next = *data->cached_rq;
-			*data->cached_rq = rq;
+			rq_list_add_tail(data->cached_rq, rq);
 		}
 		data->nr_tags -= nr;
+		return rq_list_pop(data->cached_rq);
 	} else {
 		/*
 		 * Waiting allocations only fail because of an inactive hctx.
@@ -453,14 +453,6 @@  static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 
 		return blk_mq_rq_ctx_init(data, tag, alloc_time_ns);
 	}
-
-	if (data->cached_rq) {
-		rq = *data->cached_rq;
-		*data->cached_rq = rq->rq_next;
-		return rq;
-	}
-
-	return NULL;
 }
 
 struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op,
@@ -603,11 +595,9 @@  EXPORT_SYMBOL_GPL(blk_mq_free_request);
 
 void blk_mq_free_plug_rqs(struct blk_plug *plug)
 {
-	while (plug->cached_rq) {
-		struct request *rq;
+	struct request *rq;
 
-		rq = plug->cached_rq;
-		plug->cached_rq = rq->rq_next;
+	while ((rq = rq_list_pop(&plug->cached_rq)) != NULL) {
 		percpu_ref_get(&rq->q->q_usage_counter);
 		blk_mq_free_request(rq);
 	}
@@ -2264,8 +2254,7 @@  void blk_mq_submit_bio(struct bio *bio)
 
 	plug = blk_mq_plug(q, bio);
 	if (plug && plug->cached_rq) {
-		rq = plug->cached_rq;
-		plug->cached_rq = rq->rq_next;
+		rq = rq_list_pop(&plug->cached_rq);
 		INIT_LIST_HEAD(&rq->queuelist);
 	} else {
 		struct blk_mq_alloc_data data = {
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index a9c1d0882550..c05560524841 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -473,6 +473,31 @@  struct blk_mq_tag_set {
 	struct list_head	tag_list;
 };
 
+#define rq_list_add_tail(listptr, rq)	do {		\
+	(rq)->rq_next = *(listptr);			\
+	*(listptr) = rq;				\
+} while (0)
+
+#define rq_list_pop(listptr)				\
+({							\
+	struct request *__req = NULL;			\
+	if ((listptr) && *(listptr))	{		\
+		__req = *(listptr);			\
+		*(listptr) = __req->rq_next;		\
+	}						\
+	__req;						\
+})
+
+#define rq_list_peek(listptr)				\
+({							\
+	struct request *__req = NULL;			\
+	if ((listptr) && *(listptr))			\
+		__req = *(listptr);			\
+	__req;						\
+})
+
+#define rq_list_next(rq)	(rq)->rq_next
+
 /**
  * struct blk_mq_queue_data - Data about a request inserted in a queue
  *