[PATCH 0/3] Three small block layer patches

[PATCH 0/3] Three small block layer patches

[Bart Van Assche]

Hi Jens,

This series includes two code cleanup patches and one bug fix.

Please consider the three patches in this series for inclusion in the upstream
kernel.

Thanks,

Bart.

Bart Van Assche (3):
  mq-deadline: Remove a local variable
  blk-mq: Clean up blk_mq_requeue_work()
  block: Fix queue_iostats_passthrough_show()

 block/blk-mq.c      | 10 ++++------
 block/blk-sysfs.c   |  2 +-
 block/mq-deadline.c |  5 +----
 3 files changed, 6 insertions(+), 11 deletions(-)

[PATCH 1/3] mq-deadline: Remove a local variable

[Bart Van Assche]

Since commit fde02699c242 ("block: mq-deadline: Remove support for zone
write locking"), the local variable 'insert_before' is assigned once and
is used once. Hence remove this local variable.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/mq-deadline.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 91b3789f710e..5528347b5fcf 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -698,8 +698,6 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 		list_add(&rq->queuelist, &per_prio->dispatch);
 		rq->fifo_time = jiffies;
 	} else {
-		struct list_head *insert_before;
-
 		deadline_add_rq_rb(per_prio, rq);
 
 		if (rq_mergeable(rq)) {
@@ -712,8 +710,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 		 * set expire time and add to fifo list
 		 */
 		rq->fifo_time = jiffies + dd->fifo_expire[data_dir];
-		insert_before = &per_prio->fifo_list[data_dir];
-		list_add_tail(&rq->queuelist, insert_before);
+		list_add_tail(&rq->queuelist, &per_prio->fifo_list[data_dir]);
 	}
 }
 

[PATCH 2/3] blk-mq: Clean up blk_mq_requeue_work()

[Bart Van Assche]

Move a statement that occurs in both branches of an if-statement in front
of the if-statement. Fix a typo in a source code comment. No functionality
has been changed.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a6ab1757a21a..68dbac660256 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1535,19 +1535,17 @@ static void blk_mq_requeue_work(struct work_struct *work)
 
 	while (!list_empty(&rq_list)) {
 		rq = list_entry(rq_list.next, struct request, queuelist);
+		list_del_init(&rq->queuelist);
 		/*
-		 * If RQF_DONTPREP ist set, the request has been started by the
+		 * If RQF_DONTPREP is set, the request has been started by the
 		 * driver already and might have driver-specific data allocated
 		 * already.  Insert it into the hctx dispatch list to avoid
 		 * block layer merges for the request.
 		 */
-		if (rq->rq_flags & RQF_DONTPREP) {
-			list_del_init(&rq->queuelist);
+		if (rq->rq_flags & RQF_DONTPREP)
 			blk_mq_request_bypass_insert(rq, 0);
-		} else {
-			list_del_init(&rq->queuelist);
+		else
 			blk_mq_insert_request(rq, BLK_MQ_INSERT_AT_HEAD);
-		}
 	}
 
 	while (!list_empty(&flush_list)) {

[PATCH 3/3] block: Fix queue_iostats_passthrough_show()

[Bart Van Assche]

Make queue_iostats_passthrough_show() report 0/1 in sysfs instead of 0/4.

This patch fixes the following sparse warning:
block/blk-sysfs.c:266:31: warning: incorrect type in argument 1 (different base types)
block/blk-sysfs.c:266:31:    expected unsigned long var
block/blk-sysfs.c:266:31:    got restricted blk_flags_t

Cc: Keith Busch <kbusch@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Fixes: 110234da18ab ("block: enable passthrough command statistics")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 4241aea84161..767598e719ab 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -263,7 +263,7 @@ static ssize_t queue_nr_zones_show(struct gendisk *disk, char *page)
 
 static ssize_t queue_iostats_passthrough_show(struct gendisk *disk, char *page)
 {
-	return queue_var_show(blk_queue_passthrough_stat(disk->queue), page);
+	return queue_var_show(!!blk_queue_passthrough_stat(disk->queue), page);
 }
 
 static ssize_t queue_iostats_passthrough_store(struct gendisk *disk,

Re: [PATCH 1/3] mq-deadline: Remove a local variable

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] blk-mq: Clean up blk_mq_requeue_work()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/3] block: Fix queue_iostats_passthrough_show()

[Christoph Hellwig]

On Thu, Dec 12, 2024 at 01:29:41PM -0800, Bart Van Assche wrote:
> Make queue_iostats_passthrough_show() report 0/1 in sysfs instead of 0/4.
> 
> This patch fixes the following sparse warning:
> block/blk-sysfs.c:266:31: warning: incorrect type in argument 1 (different base types)
> block/blk-sysfs.c:266:31:    expected unsigned long var
> block/blk-sysfs.c:266:31:    got restricted blk_flags_t

Maybe turn blk_queue_passthrough_stat into a an inline wrapper so
that it automatically does the bool propagation and callers don't
have to bother?

Re: [PATCH 2/3] blk-mq: Clean up blk_mq_requeue_work()

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

On 12/12/24 01:29PM, Bart Van Assche wrote:
>Move a statement that occurs in both branches of an if-statement in front
>of the if-statement. Fix a typo in a source code comment. No functionality
>has been changed.
>
>Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
>Cc: Christoph Hellwig <hch@lst.de>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>---

Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>

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

Re: [PATCH 1/3] mq-deadline: Remove a local variable

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

On 12/12/24 01:29PM, Bart Van Assche wrote:
>Since commit fde02699c242 ("block: mq-deadline: Remove support for zone
>write locking"), the local variable 'insert_before' is assigned once and
>is used once. Hence remove this local variable.
>
>Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
>Cc: Christoph Hellwig <hch@lst.de>
>Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>---

Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>

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

Re: [PATCH 1/3] mq-deadline: Remove a local variable

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 2/3] blk-mq: Clean up blk_mq_requeue_work()

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 0/3] Three small block layer patches

[Jens Axboe]

On Thu, 12 Dec 2024 13:29:38 -0800, Bart Van Assche wrote:
> This series includes two code cleanup patches and one bug fix.
> 
> Please consider the three patches in this series for inclusion in the upstream
> kernel.
> 
> Thanks,
> 
> [...]

Applied, thanks!

[1/3] mq-deadline: Remove a local variable
      commit: e01424fab35d77439956ea98c915c639fbf16db0
[2/3] blk-mq: Clean up blk_mq_requeue_work()
      commit: 312ccd4b755a09dc44e8a25f9c9526a4587ab53c
[3/3] block: Fix queue_iostats_passthrough_show()
      commit: a6fe7b70513fbf11ffa5e85f7b6ba444497a5a3d

Best regards,
-- 
Jens Axboe

Re: [PATCH 3/3] block: Fix queue_iostats_passthrough_show()

[Bart Van Assche]

On 12/12/24 8:46 PM, Christoph Hellwig wrote:
> On Thu, Dec 12, 2024 at 01:29:41PM -0800, Bart Van Assche wrote:
>> Make queue_iostats_passthrough_show() report 0/1 in sysfs instead of 0/4.
>>
>> This patch fixes the following sparse warning:
>> block/blk-sysfs.c:266:31: warning: incorrect type in argument 1 (different base types)
>> block/blk-sysfs.c:266:31:    expected unsigned long var
>> block/blk-sysfs.c:266:31:    got restricted blk_flags_t
> 
> Maybe turn blk_queue_passthrough_stat into a an inline wrapper so
> that it automatically does the bool propagation and callers don't
> have to bother?

Hi Christoph,

There are about 16 functions in include/linux/blkdev.h that test a 
single bit in q->queue_flags or q->limits.features. Do you really want
me to convert all these macros into inline functions?

Thanks,

Bart.

Re: [PATCH 3/3] block: Fix queue_iostats_passthrough_show()

[Christoph Hellwig]

On Fri, Dec 13, 2024 at 08:26:44AM -0800, Bart Van Assche wrote:
> There are about 16 functions in include/linux/blkdev.h that test a single 
> bit in q->queue_flags or q->limits.features. Do you really want
> me to convert all these macros into inline functions?

Given that all of them could leak the __bitwise type that could be
worthwhile.  Alternatively we could also take a hard look at these
helpers and check if they are even worth having and otherwise just
open code the check for the bits.