Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/1] blk-mq: Inline request status checkers
       [not found] <1cd320dad54bd78cb6721f7fe8dd2e197b9fbfa2.1569830796.git.asml.silence@gmail.com>
@ 2019-09-30 19:43 ` Pavel Begunkov (Silence)
  2019-09-30 19:53   ` Bart Van Assche
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov (Silence) @ 2019-09-30 19:43 UTC (permalink / raw)
  To: Jens Axboe, Josef Bacik, Keith Busch, Christoph Hellwig,
	Sagi Grimberg, linux-block, linux-kernel, nbd, linux-nvme
  Cc: Pavel Begunkov

From: Pavel Begunkov <asml.silence@gmail.com>

blk_mq_request_completed() and blk_mq_request_started() are
short, inline it.

v2: by Christoph suggestion inline them by hand

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 block/blk-mq-tag.c       |  4 ++--
 block/blk-mq.c           | 14 +-------------
 block/blk-mq.h           |  9 ---------
 drivers/block/nbd.c      |  2 +-
 drivers/nvme/host/core.c |  2 +-
 include/linux/blk-mq.h   | 10 ++++++++--
 6 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 008388e82b5c..32d63b607391 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -282,7 +282,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 && blk_mq_rq_state(rq) != MQ_RQ_IDLE)
 		return iter_data->fn(rq, iter_data->data, reserved);
 
 	return true;
@@ -360,7 +360,7 @@ static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
 {
 	unsigned *count = data;
 
-	if (blk_mq_request_completed(rq))
+	if (blk_mq_rq_state(rq) == MQ_RQ_COMPLETE)
 		(*count)++;
 	return true;
 }
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 29275f5a996f..a30f108d96be 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -663,18 +663,6 @@ bool blk_mq_complete_request(struct request *rq)
 }
 EXPORT_SYMBOL(blk_mq_complete_request);
 
-int blk_mq_request_started(struct request *rq)
-{
-	return blk_mq_rq_state(rq) != MQ_RQ_IDLE;
-}
-EXPORT_SYMBOL_GPL(blk_mq_request_started);
-
-int blk_mq_request_completed(struct request *rq)
-{
-	return blk_mq_rq_state(rq) == MQ_RQ_COMPLETE;
-}
-EXPORT_SYMBOL_GPL(blk_mq_request_completed);
-
 void blk_mq_start_request(struct request *rq)
 {
 	struct request_queue *q = rq->q;
@@ -718,7 +706,7 @@ static void __blk_mq_requeue_request(struct request *rq)
 	trace_block_rq_requeue(q, rq);
 	rq_qos_requeue(q, rq);
 
-	if (blk_mq_request_started(rq)) {
+	if (blk_mq_rq_state(rq) != MQ_RQ_IDLE) {
 		WRITE_ONCE(rq->state, MQ_RQ_IDLE);
 		rq->rq_flags &= ~RQF_TIMED_OUT;
 		if (q->dma_drain_size && blk_rq_bytes(rq))
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 32c62c64e6c2..eaaca8fc1c28 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -128,15 +128,6 @@ extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
 
 void blk_mq_release(struct request_queue *q);
 
-/**
- * blk_mq_rq_state() - read the current MQ_RQ_* state of a request
- * @rq: target request.
- */
-static inline enum mq_rq_state blk_mq_rq_state(struct request *rq)
-{
-	return READ_ONCE(rq->state);
-}
-
 static inline struct blk_mq_ctx *__blk_mq_get_ctx(struct request_queue *q,
 					   unsigned int cpu)
 {
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index ac07e8c94c79..71a13a65c9d1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -696,7 +696,7 @@ static struct nbd_cmd *nbd_read_stat(struct nbd_device *nbd, int index)
 	if (hwq < nbd->tag_set.nr_hw_queues)
 		req = blk_mq_tag_to_rq(nbd->tag_set.tags[hwq],
 				       blk_mq_unique_tag_to_tag(tag));
-	if (!req || !blk_mq_request_started(req)) {
+	if (!req || !blk_mq_rq_state(req) != MQ_RQ_IDLE) {
 		dev_err(disk_to_dev(nbd->disk), "Unexpected reply (%d) %p\n",
 			tag, req);
 		return ERR_PTR(-ENOENT);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 108f60b46804..3a54677dc5ad 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -292,7 +292,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved)
 				"Cancelling I/O %d", req->tag);
 
 	/* don't abort one completed request */
-	if (blk_mq_request_completed(req))
+	if (blk_mq_rq_state(req) == MQ_RQ_COMPLETE)
 		return true;
 
 	nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 0bf056de5cc3..e993ce19a8ec 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -301,9 +301,15 @@ static inline u16 blk_mq_unique_tag_to_tag(u32 unique_tag)
 	return unique_tag & BLK_MQ_UNIQUE_TAG_MASK;
 }
 
+/**
+ * blk_mq_rq_state() - read the current MQ_RQ_* state of a request
+ * @rq: target request.
+ */
+static inline enum mq_rq_state blk_mq_rq_state(struct request *rq)
+{
+	return READ_ONCE(rq->state);
+}
 
-int blk_mq_request_started(struct request *rq);
-int blk_mq_request_completed(struct request *rq);
 void blk_mq_start_request(struct request *rq);
 void blk_mq_end_request(struct request *rq, blk_status_t error);
 void __blk_mq_end_request(struct request *rq, blk_status_t error);
-- 
2.23.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] blk-mq: Inline request status checkers
  2019-09-30 19:43 ` [PATCH v2 1/1] blk-mq: Inline request status checkers Pavel Begunkov (Silence)
@ 2019-09-30 19:53   ` Bart Van Assche
  2019-09-30 20:12     ` Pavel Begunkov
  2019-09-30 20:13     ` Mike Snitzer
  0 siblings, 2 replies; 5+ messages in thread
From: Bart Van Assche @ 2019-09-30 19:53 UTC (permalink / raw)
  To: Pavel Begunkov (Silence),
	Jens Axboe, Josef Bacik, Keith Busch, Christoph Hellwig,
	Sagi Grimberg, linux-block, linux-kernel, nbd, linux-nvme

On 9/30/19 12:43 PM, Pavel Begunkov (Silence) wrote:
> @@ -282,7 +282,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 && blk_mq_rq_state(rq) != MQ_RQ_IDLE)
>  		return iter_data->fn(rq, iter_data->data, reserved);
>  
>  	return true>
> @@ -360,7 +360,7 @@ static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
>  {
>  	unsigned *count = data;
>  
> -	if (blk_mq_request_completed(rq))
> +	if (blk_mq_rq_state(rq) == MQ_RQ_COMPLETE)
>  		(*count)++;
>  	return true;
>  }

Changes like the above significantly reduce readability of the code in
the block layer core. I don't like this. I think this patch is a step
backwards instead of a step forwards.

Bart.


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] blk-mq: Inline request status checkers
  2019-09-30 19:53   ` Bart Van Assche
@ 2019-09-30 20:12     ` Pavel Begunkov
  2019-10-02 13:09       ` Jens Axboe
  2019-09-30 20:13     ` Mike Snitzer
  1 sibling, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2019-09-30 20:12 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe, Josef Bacik, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linux-kernel, nbd,
	linux-nvme

[-- Attachment #1.1.1: Type: text/plain, Size: 1056 bytes --]

On 30/09/2019 22:53, Bart Van Assche wrote:
> On 9/30/19 12:43 PM, Pavel Begunkov (Silence) wrote:
>> @@ -282,7 +282,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 && blk_mq_rq_state(rq) != MQ_RQ_IDLE)
>>  		return iter_data->fn(rq, iter_data->data, reserved);
>>  
>>  	return true>
>> @@ -360,7 +360,7 @@ static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
>>  {
>>  	unsigned *count = data;
>>  
>> -	if (blk_mq_request_completed(rq))
>> +	if (blk_mq_rq_state(rq) == MQ_RQ_COMPLETE)
>>  		(*count)++;
>>  	return true;
>>  }
> 
> Changes like the above significantly reduce readability of the code in
> the block layer core. I don't like this. I think this patch is a step
> backwards instead of a step forwards.

Yep, looks too bulky.

Jens, could you consider the first version?


-- 
Yours sincerely,
Pavel Begunkov


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 158 bytes --]

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] blk-mq: Inline request status checkers
  2019-09-30 19:53   ` Bart Van Assche
  2019-09-30 20:12     ` Pavel Begunkov
@ 2019-09-30 20:13     ` Mike Snitzer
  1 sibling, 0 replies; 5+ messages in thread
From: Mike Snitzer @ 2019-09-30 20:13 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Sagi Grimberg, linux-kernel, Josef Bacik, linux-nvme,
	Pavel Begunkov \(Silence\),
	linux-block, Keith Busch, nbd, Christoph Hellwig

On Mon, Sep 30 2019 at  3:53pm -0400,
Bart Van Assche <bvanassche@acm.org> wrote:

> On 9/30/19 12:43 PM, Pavel Begunkov (Silence) wrote:
> > @@ -282,7 +282,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 && blk_mq_rq_state(rq) != MQ_RQ_IDLE)
> >  		return iter_data->fn(rq, iter_data->data, reserved);
> >  
> >  	return true>
> > @@ -360,7 +360,7 @@ static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
> >  {
> >  	unsigned *count = data;
> >  
> > -	if (blk_mq_request_completed(rq))
> > +	if (blk_mq_rq_state(rq) == MQ_RQ_COMPLETE)
> >  		(*count)++;
> >  	return true;
> >  }
> 
> Changes like the above significantly reduce readability of the code in
> the block layer core. I don't like this. I think this patch is a step
> backwards instead of a step forwards.

I agree, not helpful.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/1] blk-mq: Inline request status checkers
  2019-09-30 20:12     ` Pavel Begunkov
@ 2019-10-02 13:09       ` Jens Axboe
  0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2019-10-02 13:09 UTC (permalink / raw)
  To: Pavel Begunkov, Bart Van Assche, Josef Bacik, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linux-kernel, nbd,
	linux-nvme

On 9/30/19 2:12 PM, Pavel Begunkov wrote:
> On 30/09/2019 22:53, Bart Van Assche wrote:
>> On 9/30/19 12:43 PM, Pavel Begunkov (Silence) wrote:
>>> @@ -282,7 +282,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 && blk_mq_rq_state(rq) != MQ_RQ_IDLE)
>>>   		return iter_data->fn(rq, iter_data->data, reserved);
>>>   
>>>   	return true>
>>> @@ -360,7 +360,7 @@ static bool blk_mq_tagset_count_completed_rqs(struct request *rq,
>>>   {
>>>   	unsigned *count = data;
>>>   
>>> -	if (blk_mq_request_completed(rq))
>>> +	if (blk_mq_rq_state(rq) == MQ_RQ_COMPLETE)
>>>   		(*count)++;
>>>   	return true;
>>>   }
>>
>> Changes like the above significantly reduce readability of the code in
>> the block layer core. I don't like this. I think this patch is a step
>> backwards instead of a step forwards.
> 
> Yep, looks too bulky.
> 
> Jens, could you consider the first version?

Yes, first one is fine, I have applied it. Thanks.

-- 
Jens Axboe


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1cd320dad54bd78cb6721f7fe8dd2e197b9fbfa2.1569830796.git.asml.silence@gmail.com>
2019-09-30 19:43 ` [PATCH v2 1/1] blk-mq: Inline request status checkers Pavel Begunkov (Silence)
2019-09-30 19:53   ` Bart Van Assche
2019-09-30 20:12     ` Pavel Begunkov
2019-10-02 13:09       ` Jens Axboe
2019-09-30 20:13     ` Mike Snitzer

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git