Linux block layer
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/3] scsi: scan: allocate sdev and starget on the NUMA node of the host adapter
From: John Garry @ 2026-04-20 12:10 UTC (permalink / raw)
  To: Sumit Saxena, martin.petersen, axboe
  Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, James Rizzo
In-Reply-To: <20260420113846.1401374-2-sumit.saxena@broadcom.com>

On 20/04/2026 12:38, Sumit Saxena wrote:
> From: James Rizzo <james.rizzo@broadcom.com>
> 
> When a host adapter is attached to a specific NUMA node, allocating
> scsi_device and scsi_target via kzalloc() may place them on a remote
> node.  All hot-path I/O accesses to these structures then cross the NUMA
> interconnect, adding latency and consuming inter-node bandwidth.
> 
> Use kzalloc_node() with dev_to_node(shost->dma_dev) so allocations land
> on the same node as the HBA, reducing cross-node traffic and improving
> I/O performance on NUMA systems.

I suppose that this makes sense. We already do this sort of thing in 
scsi_mq_setup_tags() (in setting numa node) and 
scsi_realloc_sdev_budget_map() -> 
sbitmap_init_node(sdev->request_queue->numa_node)

For the actual shost allocation, we still use kzalloc() in 
scsi_host_alloc(). However, shost associated device is often a pci 
device, and we probe pci devices in the same NUMA node it exists, and we 
try NUMA local allocations by default, so nothing is needed to change 
for the shost allocation - is this right?

> 
> Signed-off-by: James Rizzo <james.rizzo@broadcom.com>
> Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
> ---
>   drivers/scsi/scsi_scan.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef22a4228b85..9749a8dbe964 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -34,6 +34,7 @@
>   #include <linux/kthread.h>
>   #include <linux/spinlock.h>
>   #include <linux/async.h>
> +#include <linux/topology.h>f
>   #include <linux/slab.h>
>   #include <linux/unaligned.h>
>   
> @@ -286,9 +287,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
>   	int display_failure_msg = 1, ret;
>   	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
>   	struct queue_limits lim;
> +	int node = dev_to_node(shost->dma_dev);

this variable is only used once, so we can use 
dev_to_node(shost->dma_dev) directly

>   
> -	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
> -		       GFP_KERNEL);
> +	sdev = kzalloc_node(sizeof(*sdev) + shost->transportt->device_size,
> +		       GFP_KERNEL, node);
>   	if (!sdev)
>   		goto out;
>   
> @@ -501,8 +503,9 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
>   	struct scsi_target *starget;
>   	struct scsi_target *found_target;
>   	int error, ref_got;
> +	int node = dev_to_node(shost->dma_dev);

same as above

>   
> -	starget = kzalloc(size, GFP_KERNEL);
> +	starget = kzalloc_node(size, GFP_KERNEL, node);
>   	if (!starget) {
>   		printk(KERN_ERR "%s: allocation failure\n", __func__);
>   		return NULL;


^ permalink raw reply

* [PATCH 6.1.y] ublk: fix deadlock when reading partition table
From: Ruohan Lan @ 2026-04-20 12:01 UTC (permalink / raw)
  To: gregkh, sashal, stable
  Cc: linux-block, Ming Lei, Caleb Sander Mateos, Jens Axboe,
	Ruohan Lan

From: Ming Lei <ming.lei@redhat.com>

[ Upstream commit c258f5c4502c9667bccf5d76fa731ab9c96687c1 ]

When one process(such as udev) opens ublk block device (e.g., to read
the partition table via bdev_open()), a deadlock[1] can occur:

1. bdev_open() grabs disk->open_mutex
2. The process issues read I/O to ublk backend to read partition table
3. In __ublk_complete_rq(), blk_update_request() or blk_mq_end_request()
   runs bio->bi_end_io() callbacks
4. If this triggers fput() on file descriptor of ublk block device, the
   work may be deferred to current task's task work (see fput() implementation)
5. This eventually calls blkdev_release() from the same context
6. blkdev_release() tries to grab disk->open_mutex again
7. Deadlock: same task waiting for a mutex it already holds

The fix is to run blk_update_request() and blk_mq_end_request() with bottom
halves disabled. This forces blkdev_release() to run in kernel work-queue
context instead of current task work context, and allows ublk server to make
forward progress, and avoids the deadlock.

Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
Link: https://github.com/ublk-org/ublksrv/issues/170 [1]
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
[axboe: rewrite comment in ublk]
Signed-off-by: Jens Axboe <axboe@kernel.dk>
[ The fix omits the change in __ublk_do_auto_buf_reg() since this function
doesn't exist in 6.1. ]
Signed-off-by: Ruohan Lan <ruohanlan@aliyun.com>
---
 drivers/block/ublk_drv.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 121b62f8bb0a..00d29a3c3d28 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -603,12 +603,20 @@ static inline bool ubq_daemon_is_dying(struct ublk_queue *ubq)
 	return ubq->ubq_daemon->flags & PF_EXITING;
 }
 
+static void ublk_end_request(struct request *req, blk_status_t error)
+{
+	local_bh_disable();
+	blk_mq_end_request(req, error);
+	local_bh_enable();
+}
+
 /* todo: handle partial completion */
 static void ublk_complete_rq(struct request *req)
 {
 	struct ublk_queue *ubq = req->mq_hctx->driver_data;
 	struct ublk_io *io = &ubq->ios[req->tag];
 	unsigned int unmapped_bytes;
+	bool requeue;
 
 	/* failed read IO if nothing is read */
 	if (!io->res && req_op(req) == REQ_OP_READ)
@@ -641,7 +649,23 @@ static void ublk_complete_rq(struct request *req)
 	if (unlikely(unmapped_bytes < io->res))
 		io->res = unmapped_bytes;
 
-	if (blk_update_request(req, BLK_STS_OK, io->res))
+	/*
+	 * Run bio->bi_end_io() with softirqs disabled. If the final fput
+	 * happens off this path, then that will prevent ublk's blkdev_release()
+	 * from being called on current's task work, see fput() implementation.
+	 *
+	 * Otherwise, ublk server may not provide forward progress in case of
+	 * reading the partition table from bdev_open() with disk->open_mutex
+	 * held, and causes dead lock as we could already be holding
+	 * disk->open_mutex here.
+	 *
+	 * Preferably we would not be doing IO with a mutex held that is also
+	 * used for release, but this work-around will suffice for now.
+	 */
+	local_bh_disable();
+	requeue = blk_update_request(req, BLK_STS_OK, io->res);
+	local_bh_enable();
+	if (requeue)
 		blk_mq_requeue_request(req, true);
 	else
 		__blk_mq_end_request(req, BLK_STS_OK);
@@ -694,7 +718,7 @@ static inline void __ublk_abort_rq(struct ublk_queue *ubq,
 	if (ublk_queue_can_use_recovery(ubq))
 		blk_mq_requeue_request(rq, false);
 	else
-		blk_mq_end_request(rq, BLK_STS_IOERR);
+		ublk_end_request(rq, BLK_STS_IOERR);
 
 	mod_delayed_work(system_wq, &ubq->dev->monitor_work, 0);
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH v2 3/3] scsi: use percpu counters for iorequest_cnt and iodone_cnt
From: Sumit Saxena @ 2026-04-20 11:38 UTC (permalink / raw)
  To: martin.petersen, axboe
  Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, Sumit Saxena,
	Bart Van Assche
In-Reply-To: <20260420113846.1401374-1-sumit.saxena@broadcom.com>

iorequest_cnt and iodone_cnt are updated on every command dispatch and
completion, often from different CPUs on high queue depth workloads.
Using adjacent atomic_t fields caused cache line contention between the
submission and completion paths.

Represent these statistics with struct percpu_counter so increments are
mostly local to each CPU, avoiding false sharing without growing
struct scsi_device further for cache-line padding.

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
---
 drivers/scsi/scsi_error.c  |  2 +-
 drivers/scsi/scsi_lib.c    |  8 ++++----
 drivers/scsi/scsi_scan.c   |  9 +++++++++
 drivers/scsi/scsi_sysfs.c  | 27 +++++++++++++++++++++++----
 include/scsi/scsi_device.h |  5 +++--
 5 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 147127fb4db9..c7424ce92f3e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -370,7 +370,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req)
 	 */
 	if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state))
 		return BLK_EH_DONE;
-	atomic_inc(&scmd->device->iodone_cnt);
+	percpu_counter_inc(&scmd->device->iodone_cnt);
 	if (scsi_abort_command(scmd) != SUCCESS) {
 		set_host_byte(scmd, DID_TIME_OUT);
 		scsi_eh_scmd_add(scmd);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6e8c7a42603e..0b05cb63f630 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1554,7 +1554,7 @@ static void scsi_complete(struct request *rq)
 
 	INIT_LIST_HEAD(&cmd->eh_entry);
 
-	atomic_inc(&cmd->device->iodone_cnt);
+	percpu_counter_inc(&cmd->device->iodone_cnt);
 	if (cmd->result)
 		atomic_inc(&cmd->device->ioerr_cnt);
 
@@ -1592,7 +1592,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 	struct Scsi_Host *host = cmd->device->host;
 	int rtn = 0;
 
-	atomic_inc(&cmd->device->iorequest_cnt);
+	percpu_counter_inc(&cmd->device->iorequest_cnt);
 
 	/* check if the device is still usable */
 	if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
@@ -1614,7 +1614,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 		 */
 		SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
 			"queuecommand : device blocked\n"));
-		atomic_dec(&cmd->device->iorequest_cnt);
+		percpu_counter_dec(&cmd->device->iorequest_cnt);
 		return SCSI_MLQUEUE_DEVICE_BUSY;
 	}
 
@@ -1647,7 +1647,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 	trace_scsi_dispatch_cmd_start(cmd);
 	rtn = host->hostt->queuecommand(host, cmd);
 	if (rtn) {
-		atomic_dec(&cmd->device->iorequest_cnt);
+		percpu_counter_dec(&cmd->device->iorequest_cnt);
 		trace_scsi_dispatch_cmd_error(cmd, rtn);
 		if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
 		    rtn != SCSI_MLQUEUE_TARGET_BUSY)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 9749a8dbe964..0b4fa89149af 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -351,6 +351,15 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 
 	scsi_sysfs_device_initialize(sdev);
 
+	ret = percpu_counter_init(&sdev->iorequest_cnt, 0, GFP_KERNEL);
+	if (ret)
+		goto out_device_destroy;
+	ret = percpu_counter_init(&sdev->iodone_cnt, 0, GFP_KERNEL);
+	if (ret) {
+		percpu_counter_destroy(&sdev->iorequest_cnt);
+		goto out_device_destroy;
+	}
+
 	if (scsi_device_is_pseudo_dev(sdev))
 		return sdev;
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04..1f5b2dc156a8 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -516,6 +516,10 @@ static void scsi_device_dev_release(struct device *dev)
 	if (vpd_pgb7)
 		kfree_rcu(vpd_pgb7, rcu);
 	kfree(sdev->inquiry);
+	if (percpu_counter_initialized(&sdev->iodone_cnt))
+		percpu_counter_destroy(&sdev->iodone_cnt);
+	if (percpu_counter_initialized(&sdev->iorequest_cnt))
+		percpu_counter_destroy(&sdev->iorequest_cnt);
 	kfree(sdev);
 
 	if (parent)
@@ -936,11 +940,26 @@ static ssize_t
 show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
-	return snprintf(buf, 20, "%d\n", (int)sizeof(atomic_t) * 8);
+	/*
+	 * iorequest_cnt and iodone_cnt are per-CPU sums (s64); ioerr_cnt and
+	 * iotmo_cnt remain atomic_t.  Report the widest counter for tools.
+	 */
+	return snprintf(buf, 20, "%zu\n", sizeof(s64) * 8);
 }
 
 static DEVICE_ATTR(iocounterbits, S_IRUGO, show_iostat_counterbits, NULL);
 
+#define show_sdev_iostat_percpu(field)					\
+static ssize_t								\
+show_iostat_##field(struct device *dev, struct device_attribute *attr,	\
+		    char *buf)						\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	unsigned long long count = percpu_counter_sum(&sdev->field);	\
+	return snprintf(buf, 20, "0x%llx\n", count);			\
+}									\
+static DEVICE_ATTR(field, 0444, show_iostat_##field, NULL)
+
 #define show_sdev_iostat(field)						\
 static ssize_t								\
 show_iostat_##field(struct device *dev, struct device_attribute *attr,	\
@@ -950,10 +969,10 @@ show_iostat_##field(struct device *dev, struct device_attribute *attr,	\
 	unsigned long long count = atomic_read(&sdev->field);		\
 	return snprintf(buf, 20, "0x%llx\n", count);			\
 }									\
-static DEVICE_ATTR(field, S_IRUGO, show_iostat_##field, NULL)
+static DEVICE_ATTR(field, 0444, show_iostat_##field, NULL)
 
-show_sdev_iostat(iorequest_cnt);
-show_sdev_iostat(iodone_cnt);
+show_sdev_iostat_percpu(iorequest_cnt);
+show_sdev_iostat_percpu(iodone_cnt);
 show_sdev_iostat(ioerr_cnt);
 show_sdev_iostat(iotmo_cnt);
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9c2a7bbe5891..ad80b500ced9 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -8,6 +8,7 @@
 #include <linux/blk-mq.h>
 #include <scsi/scsi.h>
 #include <linux/atomic.h>
+#include <linux/percpu_counter.h>
 #include <linux/sbitmap.h>
 
 struct bsg_device;
@@ -271,8 +272,8 @@ struct scsi_device {
 	unsigned int max_device_blocked; /* what device_blocked counts down from  */
 #define SCSI_DEFAULT_DEVICE_BLOCKED	3
 
-	atomic_t iorequest_cnt;
-	atomic_t iodone_cnt;
+	struct percpu_counter iorequest_cnt;
+	struct percpu_counter iodone_cnt;
 	atomic_t ioerr_cnt;
 	atomic_t iotmo_cnt;
 
-- 
2.43.7


^ permalink raw reply related

* [PATCH v2 2/3] block: drop shared-tag fairness throttling
From: Sumit Saxena @ 2026-04-20 11:38 UTC (permalink / raw)
  To: martin.petersen, axboe
  Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, Bart Van Assche,
	Sumit Saxena
In-Reply-To: <20260420113846.1401374-1-sumit.saxena@broadcom.com>

From: Bart Van Assche <bvanassche@acm.org>

Original patch [1] by Bart Van Assche; this version is rebased onto the
current tree.  In testing it improves IOPS by roughly 16-18% by removing
the fair-sharing throttle on shared tag queues.

This patch removes the following code and structure members:
- The function hctx_may_queue().
- blk_mq_hw_ctx.nr_active and request_queue.nr_active_requests_shared_tags
  and also all the code that modifies these two member variables.

[1]: https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
---
 block/blk-core.c       |   2 -
 block/blk-mq-debugfs.c |  22 ++++++++-
 block/blk-mq-tag.c     |   4 --
 block/blk-mq.c         |  17 +------
 block/blk-mq.h         | 100 -----------------------------------------
 include/linux/blk-mq.h |   6 ---
 include/linux/blkdev.h |   2 -
 7 files changed, 22 insertions(+), 131 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 474700ffaa1c..430907b26fc4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -421,8 +421,6 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
 
 	q->node = node_id;
 
-	atomic_set(&q->nr_active_requests_shared_tags, 0);
-
 	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
 	INIT_WORK(&q->timeout_work, blk_timeout_work);
 	INIT_LIST_HEAD(&q->icq_list);
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 28167c9baa55..6ef922d7abc1 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -467,11 +467,31 @@ static int hctx_sched_tags_bitmap_show(void *data, struct seq_file *m)
 	return 0;
 }
 
+struct count_active_params {
+	struct blk_mq_hw_ctx	*hctx;
+	int			*active;
+};
+
+static bool hctx_count_active(struct request *rq, void *data)
+{
+	const struct count_active_params *params = data;
+
+	if (rq->mq_hctx == params->hctx)
+		(*params->active)++;
+
+	return true;
+}
+
 static int hctx_active_show(void *data, struct seq_file *m)
 {
 	struct blk_mq_hw_ctx *hctx = data;
+	int active = 0;
+	struct count_active_params params = { .hctx = hctx, .active = &active };
+
+	blk_mq_all_tag_iter(hctx->sched_tags ?: hctx->tags, hctx_count_active,
+			    &params);
 
-	seq_printf(m, "%d\n", __blk_mq_active_requests(hctx));
+	seq_printf(m, "%d\n", active);
 	return 0;
 }
 
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 33946cdb5716..bfd27cc6249b 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -109,10 +109,6 @@ void __blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx)
 static int __blk_mq_get_tag(struct blk_mq_alloc_data *data,
 			    struct sbitmap_queue *bt)
 {
-	if (!data->q->elevator && !(data->flags & BLK_MQ_REQ_RESERVED) &&
-			!hctx_may_queue(data->hctx, bt))
-		return BLK_MQ_NO_TAG;
-
 	if (data->shallow_depth)
 		return sbitmap_queue_get_shallow(bt, data->shallow_depth);
 	else
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9af8c3dec3f6..3c54000bc554 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -489,8 +489,6 @@ __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data)
 		}
 	} while (data->nr_tags > nr);
 
-	if (!(data->rq_flags & RQF_SCHED_TAGS))
-		blk_mq_add_active_requests(data->hctx, nr);
 	/* caller already holds a reference, add for remainder */
 	percpu_ref_get_many(&data->q->q_usage_counter, nr - 1);
 	data->nr_tags -= nr;
@@ -587,8 +585,6 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 		goto retry;
 	}
 
-	if (!(data->rq_flags & RQF_SCHED_TAGS))
-		blk_mq_inc_active_requests(data->hctx);
 	rq = blk_mq_rq_ctx_init(data, blk_mq_tags_from_data(data), tag);
 	blk_mq_rq_time_init(rq, alloc_time_ns);
 	return rq;
@@ -763,8 +759,6 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
 	tag = blk_mq_get_tag(&data);
 	if (tag == BLK_MQ_NO_TAG)
 		goto out_queue_exit;
-	if (!(data.rq_flags & RQF_SCHED_TAGS))
-		blk_mq_inc_active_requests(data.hctx);
 	rq = blk_mq_rq_ctx_init(&data, blk_mq_tags_from_data(&data), tag);
 	blk_mq_rq_time_init(rq, alloc_time_ns);
 	rq->__data_len = 0;
@@ -807,10 +801,8 @@ static void __blk_mq_free_request(struct request *rq)
 	blk_pm_mark_last_busy(rq);
 	rq->mq_hctx = NULL;
 
-	if (rq->tag != BLK_MQ_NO_TAG) {
-		blk_mq_dec_active_requests(hctx);
+	if (rq->tag != BLK_MQ_NO_TAG)
 		blk_mq_put_tag(hctx->tags, ctx, rq->tag);
-	}
 	if (sched_tag != BLK_MQ_NO_TAG)
 		blk_mq_put_tag(hctx->sched_tags, ctx, sched_tag);
 	blk_mq_sched_restart(hctx);
@@ -1188,8 +1180,6 @@ static inline void blk_mq_flush_tag_batch(struct blk_mq_hw_ctx *hctx,
 {
 	struct request_queue *q = hctx->queue;
 
-	blk_mq_sub_active_requests(hctx, nr_tags);
-
 	blk_mq_put_tags(hctx->tags, tag_array, nr_tags);
 	percpu_ref_put_many(&q->q_usage_counter, nr_tags);
 }
@@ -1875,9 +1865,6 @@ bool __blk_mq_alloc_driver_tag(struct request *rq)
 	if (blk_mq_tag_is_reserved(rq->mq_hctx->sched_tags, rq->internal_tag)) {
 		bt = &rq->mq_hctx->tags->breserved_tags;
 		tag_offset = 0;
-	} else {
-		if (!hctx_may_queue(rq->mq_hctx, bt))
-			return false;
 	}
 
 	tag = __sbitmap_queue_get(bt);
@@ -1885,7 +1872,6 @@ bool __blk_mq_alloc_driver_tag(struct request *rq)
 		return false;
 
 	rq->tag = tag + tag_offset;
-	blk_mq_inc_active_requests(rq->mq_hctx);
 	return true;
 }
 
@@ -4037,7 +4023,6 @@ blk_mq_alloc_hctx(struct request_queue *q, struct blk_mq_tag_set *set,
 	if (!zalloc_cpumask_var_node(&hctx->cpumask, gfp, node))
 		goto free_hctx;
 
-	atomic_set(&hctx->nr_active, 0);
 	if (node == NUMA_NO_NODE)
 		node = set->numa_node;
 	hctx->numa_node = node;
diff --git a/block/blk-mq.h b/block/blk-mq.h
index aa15d31aaae9..8dfb67c55f5d 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -291,70 +291,9 @@ static inline int blk_mq_get_rq_budget_token(struct request *rq)
 	return -1;
 }
 
-static inline void __blk_mq_add_active_requests(struct blk_mq_hw_ctx *hctx,
-						int val)
-{
-	if (blk_mq_is_shared_tags(hctx->flags))
-		atomic_add(val, &hctx->queue->nr_active_requests_shared_tags);
-	else
-		atomic_add(val, &hctx->nr_active);
-}
-
-static inline void __blk_mq_inc_active_requests(struct blk_mq_hw_ctx *hctx)
-{
-	__blk_mq_add_active_requests(hctx, 1);
-}
-
-static inline void __blk_mq_sub_active_requests(struct blk_mq_hw_ctx *hctx,
-		int val)
-{
-	if (blk_mq_is_shared_tags(hctx->flags))
-		atomic_sub(val, &hctx->queue->nr_active_requests_shared_tags);
-	else
-		atomic_sub(val, &hctx->nr_active);
-}
-
-static inline void __blk_mq_dec_active_requests(struct blk_mq_hw_ctx *hctx)
-{
-	__blk_mq_sub_active_requests(hctx, 1);
-}
-
-static inline void blk_mq_add_active_requests(struct blk_mq_hw_ctx *hctx,
-					      int val)
-{
-	if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
-		__blk_mq_add_active_requests(hctx, val);
-}
-
-static inline void blk_mq_inc_active_requests(struct blk_mq_hw_ctx *hctx)
-{
-	if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
-		__blk_mq_inc_active_requests(hctx);
-}
-
-static inline void blk_mq_sub_active_requests(struct blk_mq_hw_ctx *hctx,
-					      int val)
-{
-	if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
-		__blk_mq_sub_active_requests(hctx, val);
-}
-
-static inline void blk_mq_dec_active_requests(struct blk_mq_hw_ctx *hctx)
-{
-	if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
-		__blk_mq_dec_active_requests(hctx);
-}
-
-static inline int __blk_mq_active_requests(struct blk_mq_hw_ctx *hctx)
-{
-	if (blk_mq_is_shared_tags(hctx->flags))
-		return atomic_read(&hctx->queue->nr_active_requests_shared_tags);
-	return atomic_read(&hctx->nr_active);
-}
 static inline void __blk_mq_put_driver_tag(struct blk_mq_hw_ctx *hctx,
 					   struct request *rq)
 {
-	blk_mq_dec_active_requests(hctx);
 	blk_mq_put_tag(hctx->tags, rq->mq_ctx, rq->tag);
 	rq->tag = BLK_MQ_NO_TAG;
 }
@@ -396,45 +335,6 @@ static inline void blk_mq_free_requests(struct list_head *list)
 	}
 }
 
-/*
- * For shared tag users, we track the number of currently active users
- * and attempt to provide a fair share of the tag depth for each of them.
- */
-static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
-				  struct sbitmap_queue *bt)
-{
-	unsigned int depth, users;
-
-	if (!hctx || !(hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED))
-		return true;
-
-	/*
-	 * Don't try dividing an ant
-	 */
-	if (bt->sb.depth == 1)
-		return true;
-
-	if (blk_mq_is_shared_tags(hctx->flags)) {
-		struct request_queue *q = hctx->queue;
-
-		if (!test_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags))
-			return true;
-	} else {
-		if (!test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state))
-			return true;
-	}
-
-	users = READ_ONCE(hctx->tags->active_queues);
-	if (!users)
-		return true;
-
-	/*
-	 * Allow at least some tags
-	 */
-	depth = max((bt->sb.depth + users - 1) / users, 4U);
-	return __blk_mq_active_requests(hctx) < depth;
-}
-
 /* run the code block in @dispatch_ops with rcu/srcu read lock held */
 #define __blk_mq_run_dispatch_ops(q, check_sleep, dispatch_ops)	\
 do {								\
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..ccbb07559402 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -432,12 +432,6 @@ struct blk_mq_hw_ctx {
 	/** @queue_num: Index of this hardware queue. */
 	unsigned int		queue_num;
 
-	/**
-	 * @nr_active: Number of active requests. Only used when a tag set is
-	 * shared across request queues.
-	 */
-	atomic_t		nr_active;
-
 	/** @cpuhp_online: List to store request if CPU is going to die */
 	struct hlist_node	cpuhp_online;
 	/** @cpuhp_dead: List to store request if some CPU die. */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d463b9b5a0a5..0dd2a32068ec 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -561,8 +561,6 @@ struct request_queue {
 	struct timer_list	timeout;
 	struct work_struct	timeout_work;
 
-	atomic_t		nr_active_requests_shared_tags;
-
 	struct blk_mq_tags	*sched_shared_tags;
 
 	struct list_head	icq_list;
-- 
2.43.7


^ permalink raw reply related

* [PATCH v2 1/3] scsi: scan: allocate sdev and starget on the NUMA node of the host adapter
From: Sumit Saxena @ 2026-04-20 11:38 UTC (permalink / raw)
  To: martin.petersen, axboe
  Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, James Rizzo,
	Sumit Saxena
In-Reply-To: <20260420113846.1401374-1-sumit.saxena@broadcom.com>

From: James Rizzo <james.rizzo@broadcom.com>

When a host adapter is attached to a specific NUMA node, allocating
scsi_device and scsi_target via kzalloc() may place them on a remote
node.  All hot-path I/O accesses to these structures then cross the NUMA
interconnect, adding latency and consuming inter-node bandwidth.

Use kzalloc_node() with dev_to_node(shost->dma_dev) so allocations land
on the same node as the HBA, reducing cross-node traffic and improving
I/O performance on NUMA systems.

Signed-off-by: James Rizzo <james.rizzo@broadcom.com>
Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
---
 drivers/scsi/scsi_scan.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef22a4228b85..9749a8dbe964 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -34,6 +34,7 @@
 #include <linux/kthread.h>
 #include <linux/spinlock.h>
 #include <linux/async.h>
+#include <linux/topology.h>
 #include <linux/slab.h>
 #include <linux/unaligned.h>
 
@@ -286,9 +287,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	int display_failure_msg = 1, ret;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 	struct queue_limits lim;
+	int node = dev_to_node(shost->dma_dev);
 
-	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
-		       GFP_KERNEL);
+	sdev = kzalloc_node(sizeof(*sdev) + shost->transportt->device_size,
+		       GFP_KERNEL, node);
 	if (!sdev)
 		goto out;
 
@@ -501,8 +503,9 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 	struct scsi_target *starget;
 	struct scsi_target *found_target;
 	int error, ref_got;
+	int node = dev_to_node(shost->dma_dev);
 
-	starget = kzalloc(size, GFP_KERNEL);
+	starget = kzalloc_node(size, GFP_KERNEL, node);
 	if (!starget) {
 		printk(KERN_ERR "%s: allocation failure\n", __func__);
 		return NULL;
-- 
2.43.7


^ permalink raw reply related

* [PATCH v2 0/3] scsi/block: NUMA-local scan allocations, shared-tag path cleanup, and SCSI I/O counters
From: Sumit Saxena @ 2026-04-20 11:38 UTC (permalink / raw)
  To: martin.petersen, axboe
  Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, Sumit Saxena

This series contains three performance improvements targeting the SCSI
and block layers on multi-socket NUMA and heavily loaded SMP systems.

On multi-socket NUMA systems we observed extreme I/O throughput variance
of 50-60% between runs.  This series identifies and fixes two root causes:
cross-node memory accesses due to NUMA-unaware allocations in the scan
path, and false sharing between hot atomic counters in struct request_queue
and struct scsi_device.

Performance notes:

Tested on a dual-socket NUMA system (2x 32-core, 256 GB/socket) with
an mpi3mr HBA, running fio (random read, 4K, QD 64, 16 jobs, 60 s,
direct I/O).  IOPS figures are in KIOPS (thousands of IOPS):

  Configuration                    Avg KIOPS   Range (KIOPS)   Spread
  Baseline                         6,255       4,200 - 6,700   ~37%
  Baseline + all patches           7,350       7,000 - 7,700    ~10%

Key findings:

These patches combinedly reduces the observed 50-60% run-to-run variance
to under 10%, significantly improving workload predictability and
improves IOPs by 16-18%.

No functional regressions observed.

This patch series is based on Martin's for-next tree.

Changes in v2
--------------

  Patch 1 — Same functional goal as v1 patch 1: NUMA-local scsi_device /
  scsi_target allocations in the scan path so steady-state I/O does not
  habitually touch remote memory when the host has a fixed DMA/NUMA
  affinity.

  Patch 2 — Replaces v1’s ____cacheline_aligned_in_smp on
  nr_active_requests_shared_tags with shared tags removal work done by
  Bart Van Assche [1], rebased for the current tree; it removes the
  atomic counter- nr_active_request_shared_tags that motivated the v1
  false-sharing workaround and, in our testing, improves IOPS on the order
  of roughly 16–18% for the shared-tag workload exercised.
  This patch touches include/linux/blkdev.h, so needs review from
  linux-block@vger.kernel.org; an Acked-by from the block maintainer is
  requested before merging via the SCSI tree.

  Patch 3 — Replaces v1’s cache-line padding of iodone_cnt with
  percpu_counter for both iorequest_cnt and iodone_cnt, so submission and
  completion paths mostly update CPU-local state instead of bouncing a
  single cache line, without inflating struct scsi_device for SMP
  alignment.

[1]: https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/

James Rizzo (1):
  scsi: scan: allocate sdev and starget on the NUMA node of the host adapter

Bart Van Assche (1):
  block: drop shared-tag fairness throttling

Sumit Saxena (1):
  scsi: use percpu counters for iorequest_cnt and iodone_cnt

 12 files changed, 68 insertions(+), 145 deletions(-)

---
2.43.7

^ permalink raw reply

* Re: [PATCH] block: fix deadlock between blk_mq_freeze_queue and blk_mq_dispatch_list
From: Ming Lei @ 2026-04-20  7:02 UTC (permalink / raw)
  To: Michael Wu; +Cc: linux-block, linux-kernel, axboe
In-Reply-To: <5fec2f0f-97e5-2c7a-73bd-ad2ad95f2e1d@allwinnertech.com>

On Mon, Apr 20, 2026 at 02:31:14PM +0800, Michael Wu wrote:
> I'd like to add some important information:
> 
> The three processes I mentioned—Task 1838 (Back-P10-3), Task 619
> (android.hardwar), and Task 1865 (sp-control-1)—are all in an
> uninterruptible sleep state. Therefore, once Task 1865 (sp-control-1) is
> scheduled out using `preempt_schedule_notrace`, it cannot be scheduled back.
> The reason Task 1865 (sp-control-1) is in an uninterruptible sleep state is
> because `down_write` is waiting for `io_rwsem`.
> 
> My analysis of the upstream kernel code doesn't seem to have found a fix for
> this issue. This situation should theoretically exist, but I don't have a
> platform to test this low-probability behavior. However, it's certain that
> this situation occurs during I/O scheduling algorithm switching and
> concurrent F2FS write operations.
> 
> In this situation, `io_schedule_prepare` is not used. The path used in Task
> 1865 is `schedule->sched_submit_work->blk_flush_plug->blk_mq_dispatch_list`.
> 
> As you said, this method is indeed not good, but I don't have a better idea
> to handle this deadlock situation.

Now I got the idea, because blk_flush_plug() is called on a sleeping task,
that is why the preempted code block can't get run again even though it
doesn't sleep anywhere.

Can you try the following change?

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b7f77c165a6e..4217aaaa8e47 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6966,7 +6966,9 @@ static inline void sched_submit_work(struct task_struct *tsk)
         * If we are going to sleep and we have plugged IO queued,
         * make sure to submit it to avoid deadlocks.
         */
+       preempt_disable_notrace();
        blk_flush_plug(tsk->plug, true);
+       preempt_enable_no_resched_notrace();

        lock_map_release(&sched_map);
 }



thanks,
Ming

^ permalink raw reply related

* Re: [PATCH] block: fix deadlock between blk_mq_freeze_queue and blk_mq_dispatch_list
From: Michael Wu @ 2026-04-20  6:31 UTC (permalink / raw)
  To: Ming Lei; +Cc: linux-block, linux-kernel, axboe
In-Reply-To: <20260417082744.30124-1-michael@allwinnertech.com>

I'd like to add some important information:

The three processes I mentioned—Task 1838 (Back-P10-3), Task 619 
(android.hardwar), and Task 1865 (sp-control-1)—are all in an 
uninterruptible sleep state. Therefore, once Task 1865 (sp-control-1) is 
scheduled out using `preempt_schedule_notrace`, it cannot be scheduled 
back. The reason Task 1865 (sp-control-1) is in an uninterruptible sleep 
state is because `down_write` is waiting for `io_rwsem`.

My analysis of the upstream kernel code doesn't seem to have found a fix 
for this issue. This situation should theoretically exist, but I don't 
have a platform to test this low-probability behavior. However, it's 
certain that this situation occurs during I/O scheduling algorithm 
switching and concurrent F2FS write operations.

In this situation, `io_schedule_prepare` is not used. The path used in 
Task 1865 is 
`schedule->sched_submit_work->blk_flush_plug->blk_mq_dispatch_list`.

As you said, this method is indeed not good, but I don't have a better 
idea to handle this deadlock situation.

On 2026/4/17 16:27, Michael Wu wrote:
> Kernel: Linux version 6.18.16
> Platform: Android
> 
> A three-way deadlock can occur between blk_mq_freeze_queue and
> blk_mq_dispatch_list involving percpu_ref reference counting and rwsem
> synchronization:
> 
> - Task A holds io_rwsem (e.g., F2FS write path) and enters __bio_queue_enter(),
>    where it acquires percpu_ref and waits for mq_freeze_depth==0
> - Task B holds mq_freeze_depth=1 (elevator_change) and waits for
>    q_usage_counter to reach zero in blk_mq_freeze_queue_wait()
> - Task C is scheduled out via schedule() while waiting for io_rwsem.
>    Before switching, __blk_flush_plug() triggers blk_mq_dispatch_list()
>    which acquires percpu_ref via percpu_ref_get(). If preempt_schedule_notrace()
>    is triggered before percpu_ref_put(), Task C holds the reference while
>    blocked on the rwsem.
> 
> Since Task C cannot release its percpu_ref while blocked, Task B cannot
> unfreeze the queue, and Task A cannot proceed to release the io_rwsem,
> creating a circular dependency deadlock.
> 
> Change:
> Fix by disabling preemption in blk_mq_dispatch_list() when called from
> schedule() (from_sched=true), ensuring percpu_ref_get() and percpu_ref_put()
> are atomic with respect to context switches. With from_sched=true,
> blk_mq_run_hw_queue() dispatches asynchronously via kblockd, so no driver
> callbacks run in this context and preempt_disable() is safe.
> 
> Detailed scenario description:
> When process 1838 performs f2fs_submit_page_write, it obtains io_rwsem via
> f2fs_down_write_trace. When process 1865 performs f2fs_down_write_trace and
> wants to obtain io_rwsem, it needs to wait for process 1838 to release it,
> so it can only be scheduled out via schedule. Before being scheduled out,
> it clears the plug via __blk_flush_plug, so it will run to blk_mq_dispatch_list.
> Process 619 is modifying the I/O scheduling algorithm, calling elevator_change
> to set mq_freeze_depth=1. After that, blk_mq_freeze_queue_wait will wait for
> the reference count of q_usage_counter to return to zero. Coincidentally,
> process 1838 needs to wait for mq_freeze_depth=0 when it reaches
> __bio_queue_enter, so it can only wait to be woken up after q_freeze_depth=0.
> At this time, process 1865, when blk_mq_dispatch_list reaches the point where
> percpu_ref_get increments the q_usage_counter reference, and before
> percpu_ref_put, it calls preempt_schedule_notrace to schedule the process out
> due to preemption, causing q_usage_counter to never reach zero.
> 
> At this point, process 1865 depends on io_rwsem to wake up, process 1838
> depends on mq_freeze_depth=0 to wake up, and process 619 depends on
> q_usage_counter being zero to wake up and unfreeze (setting mq_freeze_depth=0),
> resulting in a deadlock between these three processes.
> 
> Stack traces from the deadlock:
> 
> Task 1838 (Back-P10-3) - holds io_rwsem, waiting for queue unfreeze:
> Call trace:
>   __switch_to+0x1a4/0x35c
>   __schedule+0x8e0/0xec4
>   schedule+0x54/0xf8
>   __bio_queue_enter+0xbc/0x19c
>   blk_mq_submit_bio+0x118/0x814
>   __submit_bio+0x9c/0x234
>   submit_bio_noacct_nocheck+0x10c/0x2d4
>   submit_bio_noacct+0x354/0x544
>   submit_bio+0x1e8/0x208
>   f2fs_submit_write_bio+0x44/0xe4
>   __submit_merged_bio+0x40/0x114
>   f2fs_submit_page_write+0x3f0/0x7e0
>   do_write_page+0x180/0x2fc
>   f2fs_outplace_write_data+0x78/0x100
>   f2fs_do_write_data_page+0x3b8/0x500
>   f2fs_write_single_data_page+0x1ac/0x6e0
>   f2fs_write_data_pages+0x838/0xdfc
>   do_writepages+0xd0/0x19c
>   filemap_write_and_wait_range+0x204/0x274
>   f2fs_commit_atomic_write+0x54/0x960
>   __f2fs_ioctl+0x2128/0x42c8
>   f2fs_ioctl+0x38/0xb4
>   __arm64_sys_ioctl+0xa0/0xf4
> 
> Task 619 (android.hardwar) - holds mq_freeze_depth=1, waiting for percpu_ref:
> Call trace:
>   __switch_to+0x1a4/0x35c
>   __schedule+0x8e0/0xec4
>   schedule+0x54/0xf8
>   blk_mq_freeze_queue_wait+0x68/0xb0
>   blk_mq_freeze_queue_nomemsave+0x68/0x7c
>   elevator_change+0x70/0x14c
>   elv_iosched_store+0x1b0/0x234
>   queue_attr_store+0xe0/0x134
>   sysfs_kf_write+0x98/0xbc
>   kernfs_fop_write_iter+0x118/0x1e8
>   vfs_write+0x2e8/0x448
>   ksys_write+0x78/0xf0
>   __arm64_sys_write+0x1c/0x2c
> 
> Task 1865 (sp-control-1) - holds percpu_ref, preempted in dispatch_list:
> Call trace:
>   __switch_to+0x1a4/0x35c
>   __schedule+0x8e0/0xec4
>   preempt_schedule_notrace+0x60/0x7c
>   blk_mq_dispatch_list+0x5c0/0x690
>   blk_mq_flush_plug_list+0x13c/0x170
>   __blk_flush_plug+0x11c/0x17c
>   schedule+0x40/0xf8
>   schedule_preempt_disabled+0x24/0x40
>   rwsem_down_write_slowpath+0x61c/0xc88
>   down_write+0x3c/0x158
>   f2fs_down_write_trace+0x30/0x84
>   f2fs_submit_page_write+0x78/0x7e0
>   do_write_page+0x180/0x2fc
>   f2fs_outplace_write_data+0x78/0x100
>   f2fs_do_write_data_page+0x3b8/0x500
>   f2fs_write_single_data_page+0x1ac/0x6e0
>   f2fs_write_data_pages+0x838/0xdfc
>   do_writepages+0xd0/0x19c
>   filemap_write_and_wait_range+0x204/0x274
>   f2fs_commit_atomic_write+0x54/0x960
>   __f2fs_ioctl+0x2128/0x42c8
>   f2fs_ioctl+0x38/0xb4
>   __arm64_sys_ioctl+0xa0/0xf4
> 
> Signed-off-by: Michael Wu <michael@allwinnertech.com>
> ---
>   block/blk-mq.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 4c5c16cce4f8f..c290bb12c1ecb 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2936,6 +2936,14 @@ static void blk_mq_dispatch_list(struct rq_list *rqs, bool from_sched)
>   	*rqs = requeue_list;
>   	trace_block_unplug(this_hctx->queue, depth, !from_sched);
>   
> +	/*
> +	 * When called from schedule(), prevent preemption and interrupts between
> +	 * ref_get and ref_put. This ensures percpu_ref_get() and percpu_ref_put()
> +	 * are atomic with respect to context switches, avoiding a deadlock with
> +	 * blk_mq_freeze_queue where a blocked task holds a percpu_ref reference.
> +	 */
> +	if (from_sched)
> +		local_irq_disable();
>   	percpu_ref_get(&this_hctx->queue->q_usage_counter);
>   	/* passthrough requests should never be issued to the I/O scheduler */
>   	if (is_passthrough) {
> @@ -2951,6 +2959,8 @@ static void blk_mq_dispatch_list(struct rq_list *rqs, bool from_sched)
>   		blk_mq_insert_requests(this_hctx, this_ctx, &list, from_sched);
>   	}
>   	percpu_ref_put(&this_hctx->queue->q_usage_counter);
> +	if (from_sched)
> +		local_irq_enable();
>   }
>   
>   static void blk_mq_dispatch_multiple_queue_requests(struct rq_list *rqs)

-- 
Regards,
Michael Wu

^ permalink raw reply

* Re: [PATCH 0/4] nbd: replace socks pointer array with xarray to eliminate queue freeze
From: Long Li @ 2026-04-20  2:46 UTC (permalink / raw)
  To: leo.lilong, josef, axboe
  Cc: linux-block, linux-kernel, yi.zhang, yangerkun, lonuxli.64
In-Reply-To: <20260327091223.4147956-1-leo.lilong@huaweicloud.com>

On Fri, Mar 27, 2026 at 05:12:19PM +0800, leo.lilong@huaweicloud.com wrote:
> From: Long Li <leo.lilong@huawei.com>
> 
> Hi,
> 
> Commit b98e762e3d ("nbd: freeze the queue while we're adding
> connections") introduced blk_mq_freeze_queue() in the add-socket path
> to prevent use-after-free when krealloc() relocates the config->socks
> array while I/O is in flight. However, freezing the queue on every
> connection setup introduces significant latency when establishing a
> large number of connections.
> 
> This series eliminates the queue freeze by replacing the
> krealloc-based struct nbd_sock **socks array with a struct xarray.
> The xarray provides RCU-safe pointer publishing: each nbd_sock is
> fully initialized before being stored via xa_store(), and concurrent
> readers access individual entries through xa_load() without ever
> holding a reference to the array itself. This removes the possibility
> of UAF on array reallocation, making the queue freeze unnecessary.
> 
> The following test was performed with 256 connections on a local
> nbd-server:
> 
>   nbd-server -M 256 -C /etc/nbd-server/config
>   time nbd-client 127.0.0.1 10809 /dev/nbd0 -N myexport -C 256
> 
>   Before:  real 4.510s  user 0.004s  sys 0.038s
>   After:   real 0.263s  user 0.009s  sys 0.032s
> 
> Connection setup time is reduced by ~94%.
> 
> Long Li (4):
>   nbd: simplify find_fallback() by removing redundant logic
>   nbd: replace socks pointer array with xarray
>   nbd: remove redundant num_connections boundary checks
>   nbd: remove queue freeze in nbd_add_socket
> 
>  drivers/block/nbd.c | 201 +++++++++++++++++++++++---------------------
>  1 file changed, 106 insertions(+), 95 deletions(-)
> 
> -- 
> 2.39.2
> 

Friendly ping ...

^ permalink raw reply

* [PATCH v3] Fix null-ptr-deref in bio_integrity_map_user()
From: Sungwoo Kim @ 2026-04-20  2:03 UTC (permalink / raw)
  To: Jens Axboe, Keith Busch
  Cc: Sungwoo Kim, Chao Shi, Weidong Zhu, Dave Tian, linux-block,
	linux-kernel

pin_user_pages_fast() can partially succeed and return the number of
pages that were actually pinned. However, the bio_integrity_map_user()
does not handle this partial pinning. This leads to a general protection
fault since bvec_from_pages() dereferences an unpinned page address,
which is 0.

To fix this, add a check to verify that all requested memory is pinned.

Reproducer in blktest: https://github.com/linux-blktests/blktests/pull/244

Kernel Oops:

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 UID: 0 PID: 1061 Comm: nvme-passthroug Not tainted 7.0.0-11783-g90957f9314e8-dirty #16 PREEMPT(lazy)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
RIP: 0010:bio_integrity_map_user.cold+0x1b0/0x9d6

Fixes: 492c5d455969 ("block: bio-integrity: directly map user buffers")
Acked-by: Chao Shi <cshi008@fiu.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Acked-by: Dave Tian <daveti@purdue.edu>
Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
V2: https://lore.kernel.org/linux-block/20260330230256.4160820-2-iam@sung-woo.kim/
V2->V3
- Added a reproducer
- V2 incorrectly assumed pin_user_pages_fast() returns pages.

 block/bio-integrity.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e79eaf047794..c8cfd15fb589 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -402,6 +402,20 @@ int bio_integrity_map_user(struct bio *bio, struct iov_iter *iter)
 					extraction_flags, &offset);
 	if (unlikely(ret < 0))
 		goto free_bvec;
+	/* Handle partial pinning. This can happen when pin_user_pages_fast()
+	 * returns fewer pages than requested
+	 */
+	if (unlikely(ret != bytes)) {
+		int npinned = DIV_ROUND_UP(offset + ret, PAGE_SIZE);
+		int i;
+
+		for (i = 0; i < npinned; i++)
+			unpin_user_page(pages[i]);
+		if (pages != stack_pages)
+			kvfree(pages);
+		ret = -EFAULT;
+		goto free_bvec;
+	}
 
 	nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset,
 				   &is_p2p);
-- 
2.47.3


^ permalink raw reply related

* [RFC] block/nvme: exploring asynchronous durability notification semantics
From: Esteban Cerutti @ 2026-04-19 21:35 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-block, linux-nvme

Hi Hannes, Christoph,

thank you both for the detailed replies. They were very helpful and
corrected several misconceptions on my side.

In particular, I now understand that:

- reliance on synchronous flushes is not universal and largely depends
  on filesystem design (e.g. ext4 vs btrfs),
- FUA already provides a per-request durability guarantee when needed,
- and that power-backed guarantees at the system level are much harder
  to make reliable than they may appear.

Based on your feedback, I took some time to read more about how this
problem is handled in practice, especially at the filesystem and
database level (e.g. journaling, WAL, group commit, batching of fsync).

It seems that the cost of durability in fsync-heavy workloads is indeed
well known, but is typically addressed at higher layers rather than in
the block layer or device interface.

So rather than defending my original idea, I would like to refine the
question.

Is there a fundamental reason why these optimizations are handled
exclusively at the filesystem / application level, instead of being
exposed (or assisted) at a lower level such as the block layer or
device interface?

For example, is it due to:

  - lack of reliable visibility into device-internal persistence state,
  - difficulty in defining correct semantics across different hardware,
  - or simply that higher layers already have enough information to
    optimize safely (making lower-level support unnecessary)?

I realize that my earlier proposal likely underestimated the complexity
involved, but I am still trying to understand whether there is a
meaningful limitation at the lower layers, or if this is primarily a
question of where the problem is best solved.

Thanks again for your time and for the insights — this has already been
very useful for understanding how these pieces fit together.

Best regards,
Esteban

^ permalink raw reply

* [PATCH] rbd: fix null-ptr-deref when device_add_disk() fails
From: Dawei Feng @ 2026-04-19  9:03 UTC (permalink / raw)
  To: idryomov
  Cc: axboe, dongsheng.yang, mcgrof, ceph-devel, linux-block,
	linux-kernel, jianhao.xu, Dawei Feng, stable, Zilin Guan

do_rbd_add() publishes the device with device_add() before calling
device_add_disk(). If device_add_disk() fails after device_add()
succeeds, the error path calls rbd_free_disk() directly and then later
falls through to rbd_dev_device_release(), which calls rbd_free_disk()
again. This double teardown can leave blk-mq cleanup operating on
invalid state and trigger a null-ptr-deref in
__blk_mq_free_map_and_rqs(), reached from blk_mq_free_tag_set().

Fix this by following the normal remove ordering: call device_del()
before rbd_dev_device_release() when device_add_disk() fails after
device_add(). That keeps the teardown sequence consistent and avoids
re-entering disk cleanup through the wrong path.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available.

We reproduced the bug on v7.0 with a real Ceph backend and a QEMU x86_64
guest booted with KASAN and CONFIG_FAILSLAB enabled. The reproducer
confines failslab injections to the __add_disk() range and injects
fail-nth while mapping an RBD image through
/sys/bus/rbd/add_single_major.

On the unpatched kernel, fail-nth=4 reliably triggered the fault:

	Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
	KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
	CPU: 0 UID: 0 PID: 273 Comm: bash Not tainted 7.0.0-01247-gd60bc1401583 #6 PREEMPT(lazy)
	Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
	RIP: 0010:__blk_mq_free_map_and_rqs+0x8c/0x240
	Code: 00 00 48 8b 6b 60 41 89 f4 49 c1 e4 03 4c 01 e5 45 85 ed 0f 85 0a 01 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 e9 48 c1 e9 03 <80> 3c 01 00 0f 85 31 01 00 00 4c 8b 6d 00 4d 85 ed 0f 84 e2 00 00
	RSP: 0018:ff1100000ab0fac8 EFLAGS: 00000246
	RAX: dffffc0000000000 RBX: ff1100000c4806a0 RCX: 0000000000000000
	RDX: 0000000000000002 RSI: 0000000000000000 RDI: ff1100000c4806f4
	RBP: 0000000000000000 R08: 0000000000000001 R09: ffe21c000189001b
	R10: ff1100000c4800df R11: ff1100006cf37be0 R12: 0000000000000000
	R13: 0000000000000000 R14: ff1100000c480700 R15: ff1100000c480004
	FS:  00007f0fbe8fe740(0000) GS:ff110000e5851000(0000) knlGS:0000000000000000
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: 00007fe53473b2e0 CR3: 0000000012eef000 CR4: 00000000007516f0
	PKRU: 55555554
	Call Trace:
	 <TASK>
	 blk_mq_free_tag_set+0x77/0x460
	 do_rbd_add+0x1446/0x2b80
	 ? __pfx_do_rbd_add+0x10/0x10
	 ? lock_acquire+0x18c/0x300
	 ? find_held_lock+0x2b/0x80
	 ? sysfs_file_kobj+0xb6/0x1b0
	 ? __pfx_sysfs_kf_write+0x10/0x10
	 kernfs_fop_write_iter+0x2f4/0x4a0
	 vfs_write+0x98e/0x1000
	 ? expand_files+0x51f/0x850
	 ? __pfx_vfs_write+0x10/0x10
	 ksys_write+0xf2/0x1d0
	 ? __pfx_ksys_write+0x10/0x10
	 do_syscall_64+0x115/0x690
	 entry_SYSCALL_64_after_hwframe+0x77/0x7f
	RIP: 0033:0x7f0fbea15907
	Code: 10 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
	RSP: 002b:00007ffe22346ea8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
	RAX: ffffffffffffffda RBX: 0000000000000058 RCX: 00007f0fbea15907
	RDX: 0000000000000058 RSI: 0000563ace6c0ef0 RDI: 0000000000000001
	RBP: 0000563ace6c0ef0 R08: 0000563ace6c0ef0 R09: 6b6435726d694141
	R10: 5250337279762f78 R11: 0000000000000246 R12: 0000000000000058
	R13: 00007f0fbeb1c780 R14: ff1100000c480700 R15: ff1100000c480004
	 </TASK>

With this fix applied, rerunning the reproducer over fail-nth=1..256
yields no KASAN reports.

Fixes: 27c97abc30e2 ("rbd: add add_disk() error handling")
Cc: stable@vger.kernel.org # 5.16+
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/block/rbd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index e7da06200c1e..d92730d8c342 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -7165,7 +7165,7 @@ static ssize_t do_rbd_add(const char *buf, size_t count)
 
 	rc = device_add_disk(&rbd_dev->dev, rbd_dev->disk, NULL);
 	if (rc)
-		goto err_out_cleanup_disk;
+		goto err_out_device_del;
 
 	spin_lock(&rbd_dev_list_lock);
 	list_add_tail(&rbd_dev->node, &rbd_dev_list);
@@ -7179,8 +7179,8 @@ static ssize_t do_rbd_add(const char *buf, size_t count)
 	module_put(THIS_MODULE);
 	return rc;
 
-err_out_cleanup_disk:
-	rbd_free_disk(rbd_dev);
+err_out_device_del:
+	device_del(&rbd_dev->dev);
 err_out_image_lock:
 	rbd_dev_image_unlock(rbd_dev);
 	rbd_dev_device_release(rbd_dev);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/2] nullblk: zoned: use lock guards
From: Ricardo H H Kojo @ 2026-04-19  5:10 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block
In-Reply-To: <20260419051009.796491-1-ricardo.kojo@ime.usp.br>

Use guard() and scoped_guard() for handling spin locks instead of manually
locking and unlocking. This prevents forgotten locks due to early exits.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
---
 drivers/block/null_blk/zoned.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 384bdce6a9b7..6f59531e9b1d 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -471,13 +471,12 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 	}
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_EMPTY:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			break;
@@ -487,7 +486,6 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_CLOSED:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			dev->nr_zones_closed--;
@@ -497,8 +495,6 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 		}
 
 		dev->nr_zones_exp_open++;
-
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	zone->cond = BLK_ZONE_COND_EXP_OPEN;
@@ -526,7 +522,7 @@ static blk_status_t null_close_zone(struct nullb_device *dev,
 	}
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_IMP_OPEN:
@@ -542,7 +538,6 @@ static blk_status_t null_close_zone(struct nullb_device *dev,
 		if (zone->wp > zone->start)
 			dev->nr_zones_closed++;
 
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	if (zone->wp == zone->start)
@@ -562,17 +557,15 @@ static blk_status_t null_finish_zone(struct nullb_device *dev,
 		return BLK_STS_IOERR;
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_FULL:
 			/* Finish operation on full is not an error */
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_OK;
 		case BLK_ZONE_COND_EMPTY:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			break;
@@ -585,13 +578,11 @@ static blk_status_t null_finish_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_CLOSED:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			dev->nr_zones_closed--;
 			break;
 		default:
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_IOERR;
 		}
 
@@ -611,7 +602,7 @@ static blk_status_t null_reset_zone(struct nullb_device *dev,
 		return BLK_STS_IOERR;
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_IMP_OPEN:
@@ -627,11 +618,9 @@ static blk_status_t null_reset_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_FULL:
 			break;
 		default:
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_IOERR;
 		}
 
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	zone->cond = BLK_ZONE_COND_EMPTY;
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 1/2] nullblk: main: use lock guards
From: Ricardo H H Kojo @ 2026-04-19  5:10 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block
In-Reply-To: <20260419051009.796491-1-ricardo.kojo@ime.usp.br>

Use guard() and scoped_guard() for handling mutex and spin locks instead of
manually locking and unlocking. This prevents forgotten locks due to early
exits and remove the need of gotos.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
---
 drivers/block/null_blk/main.c     | 86 ++++++++++++-------------------
 drivers/block/null_blk/null_blk.h |  1 +
 2 files changed, 34 insertions(+), 53 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..9714dea1ec01 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -421,25 +421,15 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
 static int nullb_apply_submit_queues(struct nullb_device *dev,
 				     unsigned int submit_queues)
 {
-	int ret;
-
-	mutex_lock(&lock);
-	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
-	mutex_unlock(&lock);
-
-	return ret;
+	guard(mutex)(&lock);
+	return nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
 }
 
 static int nullb_apply_poll_queues(struct nullb_device *dev,
 				   unsigned int poll_queues)
 {
-	int ret;
-
-	mutex_lock(&lock);
-	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
-	mutex_unlock(&lock);
-
-	return ret;
+	guard(mutex)(&lock);
+	return nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
 }
 
 NULLB_DEVICE_ATTR(size, ulong, NULL);
@@ -493,15 +483,15 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 		return ret;
 
 	ret = count;
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	if (!dev->power && newp) {
 		if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))
-			goto out;
+			return ret;
 
 		ret = null_add_dev(dev);
 		if (ret) {
 			clear_bit(NULLB_DEV_FL_UP, &dev->flags);
-			goto out;
+			return ret;
 		}
 
 		set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
@@ -515,8 +505,6 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 		clear_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
 	}
 
-out:
-	mutex_unlock(&lock);
 	return ret;
 }
 
@@ -707,10 +695,9 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item)
 	struct nullb_device *dev = to_nullb_device(item);
 
 	if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
-		mutex_lock(&lock);
+		guard(mutex)(&lock);
 		dev->power = false;
 		null_del_dev(dev->nullb);
-		mutex_unlock(&lock);
 	}
 	nullb_del_fault_config(dev);
 	config_item_put(item);
@@ -1205,7 +1192,7 @@ blk_status_t null_handle_discard(struct nullb_device *dev,
 	size_t n = nr_sectors << SECTOR_SHIFT;
 	size_t temp;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	while (n > 0) {
 		temp = min_t(size_t, n, dev->blocksize);
 		null_free_sector(nullb, sector, false);
@@ -1214,7 +1201,6 @@ blk_status_t null_handle_discard(struct nullb_device *dev,
 		sector += temp >> SECTOR_SHIFT;
 		n -= temp;
 	}
-	spin_unlock_irq(&nullb->lock);
 
 	return BLK_STS_OK;
 }
@@ -1226,7 +1212,7 @@ static blk_status_t null_handle_flush(struct nullb *nullb)
 	if (!null_cache_active(nullb))
 		return 0;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	while (true) {
 		err = null_make_cache_space(nullb,
 			nullb->dev->cache_size * 1024 * 1024);
@@ -1235,7 +1221,6 @@ static blk_status_t null_handle_flush(struct nullb *nullb)
 	}
 
 	WARN_ON(!radix_tree_empty(&nullb->dev->cache));
-	spin_unlock_irq(&nullb->lock);
 	return errno_to_blk_status(err);
 }
 
@@ -1292,7 +1277,7 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 	struct req_iterator iter;
 	struct bio_vec bvec;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	rq_for_each_segment(bvec, rq, iter) {
 		len = bvec.bv_len;
 		if (transferred_bytes + len > max_bytes)
@@ -1307,7 +1292,6 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 		if (transferred_bytes >= max_bytes)
 			break;
 	}
-	spin_unlock_irq(&nullb->lock);
 
 	return err;
 }
@@ -1592,11 +1576,11 @@ static int null_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
 	int nr = 0;
 	struct request *rq;
 
-	spin_lock(&nq->poll_lock);
-	list_splice_init(&nq->poll_list, &list);
-	list_for_each_entry(rq, &list, queuelist)
-		blk_mq_set_request_complete(rq);
-	spin_unlock(&nq->poll_lock);
+	scoped_guard(spinlock, &nq->poll_lock) {
+		list_splice_init(&nq->poll_list, &list);
+		list_for_each_entry(rq, &list, queuelist)
+			blk_mq_set_request_complete(rq);
+	}
 
 	while (!list_empty(&list)) {
 		struct nullb_cmd *cmd;
@@ -1624,14 +1608,12 @@ static enum blk_eh_timer_return null_timeout_rq(struct request *rq)
 	if (hctx->type == HCTX_TYPE_POLL) {
 		struct nullb_queue *nq = hctx->driver_data;
 
-		spin_lock(&nq->poll_lock);
-		/* The request may have completed meanwhile. */
-		if (blk_mq_request_completed(rq)) {
-			spin_unlock(&nq->poll_lock);
-			return BLK_EH_DONE;
+		scoped_guard(spinlock, &nq->poll_lock) {
+			/* The request may have completed meanwhile. */
+			if (blk_mq_request_completed(rq))
+				return BLK_EH_DONE;
+			list_del_init(&rq->queuelist);
 		}
-		list_del_init(&rq->queuelist);
-		spin_unlock(&nq->poll_lock);
 	}
 
 	pr_info("rq %p timed out\n", rq);
@@ -1692,9 +1674,9 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
 	blk_mq_start_request(rq);
 
 	if (is_poll) {
-		spin_lock(&nq->poll_lock);
-		list_add_tail(&rq->queuelist, &nq->poll_list);
-		spin_unlock(&nq->poll_lock);
+		scoped_guard(spinlock, &nq->poll_lock) {
+			list_add_tail(&rq->queuelist, &nq->poll_list);
+		}
 		return BLK_STS_OK;
 	}
 	if (cmd->fake_timeout)
@@ -2081,14 +2063,13 @@ static struct nullb *null_find_dev_by_name(const char *name)
 {
 	struct nullb *nullb = NULL, *nb;
 
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	list_for_each_entry(nb, &nullb_list, list) {
 		if (strcmp(nb->disk_name, name) == 0) {
 			nullb = nb;
 			break;
 		}
 	}
-	mutex_unlock(&lock);
 
 	return nullb;
 }
@@ -2101,10 +2082,9 @@ static int null_create_dev(void)
 	dev = null_alloc_dev();
 	if (!dev)
 		return -ENOMEM;
-
-	mutex_lock(&lock);
-	ret = null_add_dev(dev);
-	mutex_unlock(&lock);
+	scoped_guard(mutex, &lock) {
+		ret = null_add_dev(dev);
+	}
 	if (ret) {
 		null_free_dev(dev);
 		return ret;
@@ -2202,12 +2182,12 @@ static void __exit null_exit(void)
 
 	unregister_blkdev(null_major, "nullb");
 
-	mutex_lock(&lock);
-	while (!list_empty(&nullb_list)) {
-		nullb = list_entry(nullb_list.next, struct nullb, list);
-		null_destroy_dev(nullb);
+	scoped_guard(mutex, &lock) {
+		while (!list_empty(&nullb_list)) {
+			nullb = list_entry(nullb_list.next, struct nullb, list);
+			null_destroy_dev(nullb);
+		}
 	}
-	mutex_unlock(&lock);
 
 	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 6c4c4bbe7dad..c2bb085d3582 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -14,6 +14,7 @@
 #include <linux/fault-inject.h>
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
+#include <linux/cleanup.h>
 
 struct nullb_cmd {
 	blk_status_t error;
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 0/2] nullblk: use lock guards
From: Ricardo H H Kojo @ 2026-04-19  5:10 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block

Use guard() and scoped_guard() for handling mutex and spin locks instead of
manually locking and unlocking in main.c and zoned.c. This prevents forgotten
locks due to early exits and remove the need of gotos.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>

Ricardo H H Kojo (2):
  nullblk: main: use lock guards
  nullblk: zoned: use lock guards

 drivers/block/null_blk/main.c     | 86 ++++++++++++-------------------
 drivers/block/null_blk/null_blk.h |  1 +
 drivers/block/null_blk/zoned.c    | 19 ++-----
 3 files changed, 38 insertions(+), 68 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v4 2/2] blk-mq: expose tag starvation counts via debugfs
From: Aaron Tomlin @ 2026-04-19  2:30 UTC (permalink / raw)
  To: axboe, rostedt, mhiramat, mathieu.desnoyers
  Cc: bvanassche, johannes.thumshirn, kch, dlemoal, ritesh.list,
	loberman, neelx, sean, mproche, chjohnst, linux-block,
	linux-kernel, linux-trace-kernel
In-Reply-To: <20260419023036.1419514-1-atomlin@atomlin.com>

In high-performance storage environments, particularly when utilising
RAID controllers with shared tag sets (BLK_MQ_F_TAG_HCTX_SHARED), severe
latency spikes can occur when fast devices are starved of available
tags.

This patch introduces two new debugfs attributes for each block
hardware queue:
  - /sys/kernel/debug/block/[device]/hctxN/wait_on_hw_tag
  - /sys/kernel/debug/block/[device]/hctxN/wait_on_sched_tag

These files expose atomic counters that increment each time a submitting
context is forced into an uninterruptible sleep via io_schedule() due to
the complete exhaustion of physical driver tags or software scheduler
tags, respectively.

To ensure negligible performance overhead even in production
environments where CONFIG_BLK_DEBUG_FS is actively enabled, this
tracking logic utilises dynamically allocated per-CPU counters. When
this configuration is disabled, the tracking logic compiles down to a
safe no-op.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 block/blk-mq-debugfs.c | 84 ++++++++++++++++++++++++++++++++++++++++++
 block/blk-mq-debugfs.h |  7 ++++
 block/blk-mq-tag.c     |  4 ++
 include/linux/blk-mq.h | 12 ++++++
 4 files changed, 107 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 047ec887456b..a3effed55d90 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -7,6 +7,7 @@
 #include <linux/blkdev.h>
 #include <linux/build_bug.h>
 #include <linux/debugfs.h>
+#include <linux/percpu.h>
 
 #include "blk.h"
 #include "blk-mq.h"
@@ -484,6 +485,54 @@ static int hctx_dispatch_busy_show(void *data, struct seq_file *m)
 	return 0;
 }
 
+/**
+ * hctx_wait_on_hw_tag_show - display hardware tag starvation count
+ * @data: generic pointer to the associated hardware context (hctx)
+ * @m: seq_file pointer for debugfs output formatting
+ *
+ * Prints the cumulative number of times a submitting context was forced
+ * to block due to the exhaustion of physical hardware driver tags.
+ *
+ * Return: 0 on success.
+ */
+static int hctx_wait_on_hw_tag_show(void *data, struct seq_file *m)
+{
+	struct blk_mq_hw_ctx *hctx = data;
+	unsigned long count = 0;
+	int cpu;
+
+	if (hctx->wait_on_hw_tag) {
+		for_each_possible_cpu(cpu)
+			count += *per_cpu_ptr(hctx->wait_on_hw_tag, cpu);
+	}
+	seq_printf(m, "%lu\n", count);
+	return 0;
+}
+
+/**
+ * hctx_wait_on_sched_tag_show - display scheduler tag starvation count
+ * @data: generic pointer to the associated hardware context (hctx)
+ * @m: seq_file pointer for debugfs output formatting
+ *
+ * Prints the cumulative number of times a submitting context was forced
+ * to block due to the exhaustion of software scheduler tags.
+ *
+ * Return: 0 on success.
+ */
+static int hctx_wait_on_sched_tag_show(void *data, struct seq_file *m)
+{
+	struct blk_mq_hw_ctx *hctx = data;
+	unsigned long count = 0;
+	int cpu;
+
+	if (hctx->wait_on_sched_tag) {
+		for_each_possible_cpu(cpu)
+			count += *per_cpu_ptr(hctx->wait_on_sched_tag, cpu);
+	}
+	seq_printf(m, "%lu\n", count);
+	return 0;
+}
+
 #define CTX_RQ_SEQ_OPS(name, type)					\
 static void *ctx_##name##_rq_list_start(struct seq_file *m, loff_t *pos) \
 	__acquires(&ctx->lock)						\
@@ -599,6 +648,8 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_attrs[] = {
 	{"active", 0400, hctx_active_show},
 	{"dispatch_busy", 0400, hctx_dispatch_busy_show},
 	{"type", 0400, hctx_type_show},
+	{"wait_on_hw_tag", 0400, hctx_wait_on_hw_tag_show},
+	{"wait_on_sched_tag", 0400, hctx_wait_on_sched_tag_show},
 	{},
 };
 
@@ -670,6 +721,11 @@ void blk_mq_debugfs_register_hctx(struct request_queue *q,
 	snprintf(name, sizeof(name), "hctx%u", hctx->queue_num);
 	hctx->debugfs_dir = debugfs_create_dir(name, q->debugfs_dir);
 
+	if (!hctx->wait_on_hw_tag)
+		hctx->wait_on_hw_tag = alloc_percpu(unsigned long);
+	if (!hctx->wait_on_sched_tag)
+		hctx->wait_on_sched_tag = alloc_percpu(unsigned long);
+
 	debugfs_create_files(q, hctx->debugfs_dir, hctx,
 			     blk_mq_debugfs_hctx_attrs);
 
@@ -684,6 +740,11 @@ void blk_mq_debugfs_unregister_hctx(struct blk_mq_hw_ctx *hctx)
 	debugfs_remove_recursive(hctx->debugfs_dir);
 	hctx->sched_debugfs_dir = NULL;
 	hctx->debugfs_dir = NULL;
+
+	free_percpu(hctx->wait_on_hw_tag);
+	hctx->wait_on_hw_tag = NULL;
+	free_percpu(hctx->wait_on_sched_tag);
+	hctx->wait_on_sched_tag = NULL;
 }
 
 void blk_mq_debugfs_register_hctxs(struct request_queue *q)
@@ -815,3 +876,26 @@ void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx)
 	debugfs_remove_recursive(hctx->sched_debugfs_dir);
 	hctx->sched_debugfs_dir = NULL;
 }
+
+/**
+ * blk_mq_debugfs_inc_wait_tags - increment the tag starvation counters
+ * @hctx: hardware context associated with the tag allocation
+ * @is_sched: true if the starved pool is the software scheduler
+ *
+ * Evaluates the exhausted tag pool and safely increments the appropriate
+ * per-cpu debugfs starvation counter.
+ *
+ * Note: A race window exists during rapid device probe or CPU hotplug
+ * where I/O might be submitted before blk_mq_debugfs_register_hctx() has
+ * completed allocating the per-CPU counters. Therefore, the pointer is
+ * explicitly checked to prevent a NULL pointer dereference.
+ */
+void blk_mq_debugfs_inc_wait_tags(struct blk_mq_hw_ctx *hctx,
+				  bool is_sched)
+{
+	unsigned long __percpu *tags = is_sched ? hctx->wait_on_sched_tag :
+						  hctx->wait_on_hw_tag;
+
+	if (likely(tags))
+		this_cpu_inc(*tags);
+}
diff --git a/block/blk-mq-debugfs.h b/block/blk-mq-debugfs.h
index 49bb1aaa83dc..a0094d004d08 100644
--- a/block/blk-mq-debugfs.h
+++ b/block/blk-mq-debugfs.h
@@ -17,6 +17,8 @@ struct blk_mq_debugfs_attr {
 	const struct seq_operations *seq_ops;
 };
 
+void blk_mq_debugfs_inc_wait_tags(struct blk_mq_hw_ctx *hctx,
+				  bool is_sched);
 int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq);
 int blk_mq_debugfs_rq_show(struct seq_file *m, void *v);
 
@@ -35,6 +37,11 @@ void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx);
 
 void blk_mq_debugfs_register_rq_qos(struct request_queue *q);
 #else
+static inline void blk_mq_debugfs_inc_wait_tags(struct blk_mq_hw_ctx *hctx,
+						bool is_sched)
+{
+}
+
 static inline void blk_mq_debugfs_register(struct request_queue *q)
 {
 }
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 66138dd043d4..3cc6a97a87a0 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -17,6 +17,7 @@
 #include "blk.h"
 #include "blk-mq.h"
 #include "blk-mq-sched.h"
+#include "blk-mq-debugfs.h"
 
 /*
  * Recalculate wakeup batch when tag is shared by hctx.
@@ -191,6 +192,9 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		trace_block_rq_tag_wait(data->q, data->hctx,
 					data->rq_flags & RQF_SCHED_TAGS);
 
+		blk_mq_debugfs_inc_wait_tags(data->hctx,
+					     data->rq_flags & RQF_SCHED_TAGS);
+
 		bt_prev = bt;
 		io_schedule();
 
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index ebc45557aee8..17cd6221bb93 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -453,6 +453,18 @@ struct blk_mq_hw_ctx {
 	struct dentry		*debugfs_dir;
 	/** @sched_debugfs_dir:	debugfs directory for the scheduler. */
 	struct dentry		*sched_debugfs_dir;
+	/**
+	 * @wait_on_hw_tag: Cumulative per-cpu counter incremented each
+	 * time a submitting context is forced to block due to physical
+	 * hardware tag exhaustion.
+	 */
+	unsigned long __percpu	*wait_on_hw_tag;
+	/**
+	 * @wait_on_sched_tag: Cumulative per-cpu counter incremented each
+	 * time a submitting context is forced to block due to software
+	 * scheduler tag exhaustion.
+	 */
+	unsigned long __percpu	*wait_on_sched_tag;
 #endif
 
 	/**
-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 1/2] blk-mq: add tracepoint block_rq_tag_wait
From: Aaron Tomlin @ 2026-04-19  2:30 UTC (permalink / raw)
  To: axboe, rostedt, mhiramat, mathieu.desnoyers
  Cc: bvanassche, johannes.thumshirn, kch, dlemoal, ritesh.list,
	loberman, neelx, sean, mproche, chjohnst, linux-block,
	linux-kernel, linux-trace-kernel
In-Reply-To: <20260419023036.1419514-1-atomlin@atomlin.com>

In high-performance storage environments, particularly when utilising
RAID controllers with shared tag sets (BLK_MQ_F_TAG_HCTX_SHARED), severe
latency spikes can occur when fast devices (SSDs) are starved of hardware
tags when sharing the same blk_mq_tag_set.

Currently, diagnosing this specific hardware queue contention is
difficult. When a CPU thread exhausts the tag pool, blk_mq_get_tag()
forces the current thread to block uninterruptible via io_schedule().
While this can be inferred via sched:sched_switch or dynamically
traced by attaching a kprobe to blk_mq_mark_tag_wait(), there is no
dedicated, out-of-the-box observability for this event.

This patch introduces the block_rq_tag_wait trace point in the tag
allocation slow-path. It triggers immediately before the thread yields
the CPU, exposing the exact hardware context (hctx) that is starved, the
specific pool experiencing starvation (hardware or software scheduler),
and the total pool depth.

This provides storage engineers and performance monitoring agents
with a zero-configuration, low-overhead mechanism to definitively
identify shared-tag bottlenecks and tune I/O schedulers or cgroup
throttling accordingly.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 block/blk-mq-tag.c           |  4 ++++
 include/trace/events/block.h | 43 ++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 33946cdb5716..66138dd043d4 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -13,6 +13,7 @@
 #include <linux/kmemleak.h>
 
 #include <linux/delay.h>
+#include <trace/events/block.h>
 #include "blk.h"
 #include "blk-mq.h"
 #include "blk-mq-sched.h"
@@ -187,6 +188,9 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		if (tag != BLK_MQ_NO_TAG)
 			break;
 
+		trace_block_rq_tag_wait(data->q, data->hctx,
+					data->rq_flags & RQF_SCHED_TAGS);
+
 		bt_prev = bt;
 		io_schedule();
 
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 6aa79e2d799c..71554b94e4d0 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -226,6 +226,49 @@ DECLARE_EVENT_CLASS(block_rq,
 		  IOPRIO_PRIO_LEVEL(__entry->ioprio), __entry->comm)
 );
 
+/**
+ * block_rq_tag_wait - triggered when a request is starved of a tag
+ * @q: request queue of the target device
+ * @hctx: hardware context of the request experiencing starvation
+ * @is_sched_tag: indicates whether the starved pool is the software scheduler
+ *
+ * Called immediately before the submitting context is forced to block due
+ * to the exhaustion of available tags (i.e., physical hardware driver tags
+ * or software scheduler tags). This trace point indicates that the context
+ * will be placed into an uninterruptible state via io_schedule() until an
+ * active request completes and relinquishes its assigned tag.
+ */
+TRACE_EVENT(block_rq_tag_wait,
+
+	TP_PROTO(struct request_queue *q, struct blk_mq_hw_ctx *hctx, bool is_sched_tag),
+
+	TP_ARGS(q, hctx, is_sched_tag),
+
+	TP_STRUCT__entry(
+		__field( dev_t,		dev			)
+		__field( u32,		hctx_id			)
+		__field( u32,		nr_tags			)
+		__field( bool,		is_sched_tag		)
+	),
+
+	TP_fast_assign(
+		__entry->dev		= disk_devt(q->disk);
+		__entry->hctx_id	= hctx->queue_num;
+		__entry->is_sched_tag	= is_sched_tag;
+
+		if (is_sched_tag)
+			__entry->nr_tags = hctx->sched_tags->nr_tags;
+		else
+			__entry->nr_tags = hctx->tags->nr_tags;
+	),
+
+	TP_printk("%d,%d hctx=%u starved on %s tags (depth=%u)",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->hctx_id,
+		  __entry->is_sched_tag ? "scheduler" : "hardware",
+		  __entry->nr_tags)
+);
+
 /**
  * block_rq_insert - insert block operation request into queue
  * @rq: block IO operation request
-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 0/2] blk-mq: introduce tag starvation observability
From: Aaron Tomlin @ 2026-04-19  2:30 UTC (permalink / raw)
  To: axboe, rostedt, mhiramat, mathieu.desnoyers
  Cc: bvanassche, johannes.thumshirn, kch, dlemoal, ritesh.list,
	loberman, neelx, sean, mproche, chjohnst, linux-block,
	linux-kernel, linux-trace-kernel

Hi Jens, Steve, Masami,

In high-performance storage environments, particularly when utilising RAID
controllers with shared tag sets (BLK_MQ_F_TAG_HCTX_SHARED), severe latency
spikes can occur when fast devices are starved of available tags.
Currently, diagnosing this specific queue contention requires deploying
dynamic kprobes or inferring sleep states, which lacks a simple,
out-of-the-box diagnostic path.

This short series introduces dedicated, low-overhead observability for tag
exhaustion events in the block layer:

  - Patch 1 introduces the "block_rq_tag_wait" tracepoint in the tag
    allocation slow-path to capture precise, event-based starvation.

  - Patch 2 complements this by exposing "wait_on_hw_tag" and
    "wait_on_sched_tag" per-CPU counters via debugfs for quick,
    point-in-time cumulative polling.

Together, these provide storage engineers with zero-configuration
mechanisms to definitively identify shared-tag bottlenecks.

Please let me know your thoughts.


Changes since v3 [1]:
 - Transitioned tracking architecture from shared atomic_t variables to
   dynamically allocated per-CPU counters to resolve cache line bouncing
   (Bart Van Assche)

Changes since v2 [2]:
 - Added "Reviewed-by:" and "Tested-by:" tags for patch 1
 - Evaluate is_sched_tag directly within TP_fast_assign (Steven Rostedt)
 - Introduced atomic counters via debugfs 

Changes since v1 [3]:
 - Improved the description of the trace point (Damien Le Moal)
 - Removed the redundant "active requests" (Laurence Oberman)
 - Introduced pool-specific starvation tracking

[1]: https://lore.kernel.org/lkml/20260319221956.332770-1-atomlin@atomlin.com/
[2]: https://lore.kernel.org/lkml/20260319015300.287653-1-atomlin@atomlin.com/
[3]: https://lore.kernel.org/lkml/20260317182835.258183-1-atomlin@atomlin.com/

Aaron Tomlin (2):
  blk-mq: add tracepoint block_rq_tag_wait
  blk-mq: expose tag starvation counts via debugfs

 block/blk-mq-debugfs.c       | 84 ++++++++++++++++++++++++++++++++++++
 block/blk-mq-debugfs.h       |  7 +++
 block/blk-mq-tag.c           |  8 ++++
 include/linux/blk-mq.h       | 12 ++++++
 include/trace/events/block.h | 43 ++++++++++++++++++
 5 files changed, 154 insertions(+)

-- 
2.51.0


^ permalink raw reply

* Re: [PATCH 1/2] nullblk: main: use lock guards
From: Jens Axboe @ 2026-04-18 13:58 UTC (permalink / raw)
  To: Ricardo H H Kojo; +Cc: Ellian Carlos, Gabriel B L de Oliveira, linux-block
In-Reply-To: <20260418043837.721045-2-ricardo.kojo@ime.usp.br>

On 4/17/26 10:38 PM, Ricardo H H Kojo wrote:
> Use guard() and scoped_guard() for handling mutex and spin locks instead of
> manually locking and unlocking. This prevents forgotten locks due to early
> exits and remove the need of gotos.
> 
> Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
> Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
> Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
> Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
> Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
> ---
>  drivers/block/null_blk/main.c     | 74 +++++++++++++------------------
>  drivers/block/null_blk/null_blk.h |  1 +
>  2 files changed, 32 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index f8c0fd57e041..fbe34e8a6c93 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -423,9 +423,8 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>  {
>  	int ret;
>  
> -	mutex_lock(&lock);
> +	guard(mutex)(&lock);
>  	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
> -	mutex_unlock(&lock);
>  
>  	return ret;
>  }

If you're going to use lock guards instead, why on earth is this not
just:

{
	guard(mutex)(&lock);
	return nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
}

?!

This odd intermediate state you opted for makes no sense.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH v2] floppy: annotate data-races around command_status and floppy_work_fn
From: Denis Efremov (Oracle) @ 2026-04-18 10:56 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, baijiaju1990, Cen Zhang
In-Reply-To: <20260318034810.3089217-1-zzzccc427@gmail.com>

Hello,

Jens, can you also take this patch to your tree?

Thanks,
Denis

On 18/03/2026 07:48, Cen Zhang wrote:
> command_status is accessed by multiple contexts:
>  - generic_done() and do_wakeup() write it from the floppy
>    workqueue context.
>  - wait_til_done() reads it in wait_event conditions without
>    holding fdc_busy.
>  - is_alive() reads it locklessly to check driver liveness.
>  - floppy_queue_rq(), lock_fdc(), and unlock_fdc() write it
>    during FDC ownership transitions.
> 
> There is currently no LKMM annotation for these concurrent accesses
> since command_status relies on the deprecated volatile qualifier.
> Remove volatile and use READ_ONCE()/WRITE_ONCE() on every access
> to command_status to provide proper LKMM data-race annotations.
> 
> Also annotate floppy_work_fn with WRITE_ONCE() in schedule_bh()
> and READ_ONCE() in floppy_work_workfn(), and current_type[drive]
> with READ_ONCE() in drive_no_geom() where it can be read without
> holding the FDC lock.
> 
> The races were found by our tools:
> 
>  1) command_status: generic_done (write) vs wait_til_done (read)
> 
>   ============ DATARACE ============
>   Function: generic_done+0x54/0xa0 drivers/block/floppy.c:985
>   Function: success_and_wakeup+0xeb/0x1e0 drivers/block/floppy.c:2046
>   Function: setup_rw_floppy+0x15dc/0x1d80 drivers/block/floppy.c:1046
>   Function: floppy_ready+0x2630/0x4000
>   Function: floppy_work_workfn+0x56/0x70 drivers/block/floppy.c:1012
>   Function: process_one_work kernel/workqueue.c:3236 [inline]
>   Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
>   ============OTHER_INFO============
>   Function: wait_til_done+0x134/0x740 drivers/block/floppy.c:2026
>   Function: poll_drive+0x1fa/0x2a0
>   Function: fd_ioctl+0x13c8/0x1da0 drivers/block/floppy.c:3873
>   Function: blkdev_ioctl+0x421/0x510 block/ioctl.c:705
>   Function: vfs_ioctl fs/ioctl.c:51 [inline]
>   Function: __do_sys_ioctl fs/ioctl.c:598 [inline]
>   Function: __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
>   =================END==============
> 
>  2) current_type[drive]: drive_no_geom (read) vs set_geometry (write)
> 
>   ============ DATARACE ============
>   Function: drive_no_geom drivers/block/floppy.c:606 [inline]
>   Function: floppy_check_events+0x9c9/0xcf0 drivers/block/floppy.c:4187
>   Function: disk_check_events+0xca/0x4d0 block/disk-events.c:193
>   Function: process_one_work kernel/workqueue.c:3236 [inline]
>   Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
>   Function: worker_thread+0xb97/0x11d0 kernel/workqueue.c:3400
>   Function: kthread+0x3d4/0x800 kernel/kthread.c:463
>   ============OTHER_INFO============
>   Function: set_geometry+0xe1d/0x1980 drivers/block/floppy.c:2237
>   Function: fd_ioctl+0xff4/0x1da0 drivers/block/floppy.c:3912
>   Function: blkdev_ioctl+0x421/0x510 block/ioctl.c:705
>   Function: vfs_ioctl fs/ioctl.c:51 [inline]
>   Function: __do_sys_ioctl fs/ioctl.c:598 [inline]
>   Function: __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
>   =================END==============
> 
>  3) floppy_work_fn: schedule_bh (write) vs
>     floppy_interrupt->schedule_bh (write)
> 
>   ============ DATARACE ============
>   Function: schedule_bh drivers/block/floppy.c:1005 [inline]
>   Function: do_wakeup+0x19c/0x220 drivers/block/floppy.c:3219
>   Function: success_and_wakeup+0x192/0x1e0 drivers/block/floppy.c:1994
>   Function: setup_rw_floppy+0x15dc/0x1d80 drivers/block/floppy.c:1046
>   Function: floppy_ready+0x2630/0x4000
>   Function: floppy_work_workfn+0x56/0x70 drivers/block/floppy.c:1012
>   Function: process_one_work kernel/workqueue.c:3236 [inline]
>   Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
>   ============OTHER_INFO============
>   Function: floppy_interrupt+0xbf4/0x1220
>   Function: floppy_hardint+0x432/0x630
>   Function: __handle_irq_event_percpu+0x10a/0x5d0 kernel/irq/handle.c:158
>   Function: handle_irq_event+0x8b/0x1e0 kernel/irq/handle.c:210
>   Function: handle_edge_irq+0x223/0x9f0 kernel/irq/chip.c:855
>   =================END==============
> 
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  drivers/block/floppy.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 92e446a64371..cf8246e7155e 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -502,7 +502,7 @@ static int probing;
>  #define FD_COMMAND_ERROR	2
>  #define FD_COMMAND_OKAY		3
>  
> -static volatile int command_status = FD_COMMAND_NONE;
> +static int command_status = FD_COMMAND_NONE;
>  static unsigned long fdc_busy;
>  static DECLARE_WAIT_QUEUE_HEAD(fdc_wait);
>  static DECLARE_WAIT_QUEUE_HEAD(command_done);
> @@ -601,7 +601,8 @@ static inline void fdc_outb(unsigned char value, int fdc, int reg)
>  
>  static inline bool drive_no_geom(int drive)
>  {
> -	return !current_type[drive] && !ITYPE(drive_state[drive].fd_device);
> +	return !READ_ONCE(current_type[drive]) &&
> +	       !ITYPE(drive_state[drive].fd_device);
>  }
>  
>  #ifndef fd_eject
> @@ -640,7 +641,7 @@ static const char *timeout_message;
>  static void is_alive(const char *func, const char *message)
>  {
>  	/* this routine checks whether the floppy driver is "alive" */
> -	if (test_bit(0, &fdc_busy) && command_status < 2 &&
> +	if (test_bit(0, &fdc_busy) && READ_ONCE(command_status) < 2 &&
>  	    !delayed_work_pending(&fd_timeout)) {
>  		DPRINT("%s: timeout handler died.  %s\n", func, message);
>  	}
> @@ -892,7 +893,7 @@ static int lock_fdc(int drive)
>  	if (wait_event_interruptible(fdc_wait, !test_and_set_bit(0, &fdc_busy)))
>  		return -EINTR;
>  
> -	command_status = FD_COMMAND_NONE;
> +	WRITE_ONCE(command_status, FD_COMMAND_NONE);
>  
>  	reschedule_timeout(drive, "lock fdc");
>  	set_fdc(drive);
> @@ -906,7 +907,7 @@ static void unlock_fdc(void)
>  		DPRINT("FDC access conflict!\n");
>  
>  	raw_cmd = NULL;
> -	command_status = FD_COMMAND_NONE;
> +	WRITE_ONCE(command_status, FD_COMMAND_NONE);
>  	cancel_delayed_work(&fd_timeout);
>  	do_floppy = NULL;
>  	cont = NULL;
> @@ -990,7 +991,9 @@ static void (*floppy_work_fn)(void);
>  
>  static void floppy_work_workfn(struct work_struct *work)
>  {
> -	floppy_work_fn();
> +	void (*fn)(void) = READ_ONCE(floppy_work_fn);
> +
> +	fn();
>  }
>  
>  static DECLARE_WORK(floppy_work, floppy_work_workfn);
> @@ -999,7 +1002,7 @@ static void schedule_bh(void (*handler)(void))
>  {
>  	WARN_ON(work_pending(&floppy_work));
>  
> -	floppy_work_fn = handler;
> +	WRITE_ONCE(floppy_work_fn, handler);
>  	queue_work(floppy_wq, &floppy_work);
>  }
>  
> @@ -1864,7 +1867,7 @@ static void show_floppy(int fdc)
>  
>  	pr_info("cont=%p\n", cont);
>  	pr_info("current_req=%p\n", current_req);
> -	pr_info("command_status=%d\n", command_status);
> +	pr_info("command_status=%d\n", READ_ONCE(command_status));
>  	pr_info("\n");
>  }
>  
> @@ -1991,7 +1994,7 @@ static void do_wakeup(void)
>  {
>  	reschedule_timeout(MAXTIMEOUT, "do wakeup");
>  	cont = NULL;
> -	command_status += 2;
> +	WRITE_ONCE(command_status, READ_ONCE(command_status) + 2);
>  	wake_up(&command_done);
>  }
>  
> @@ -2019,11 +2022,12 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
>  	schedule_bh(handler);
>  
>  	if (interruptible)
> -		wait_event_interruptible(command_done, command_status >= 2);
> +		wait_event_interruptible(command_done,
> +					 READ_ONCE(command_status) >= 2);
>  	else
> -		wait_event(command_done, command_status >= 2);
> +		wait_event(command_done, READ_ONCE(command_status) >= 2);
>  
> -	if (command_status < 2) {
> +	if (READ_ONCE(command_status) < 2) {
>  		cancel_activity();
>  		cont = &intr_cont;
>  		reset_fdc();
> @@ -2031,18 +2035,18 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
>  	}
>  
>  	if (fdc_state[current_fdc].reset)
> -		command_status = FD_COMMAND_ERROR;
> -	if (command_status == FD_COMMAND_OKAY)
> +		WRITE_ONCE(command_status, FD_COMMAND_ERROR);
> +	if (READ_ONCE(command_status) == FD_COMMAND_OKAY)
>  		ret = 0;
>  	else
>  		ret = -EIO;
> -	command_status = FD_COMMAND_NONE;
> +	WRITE_ONCE(command_status, FD_COMMAND_NONE);
>  	return ret;
>  }
>  
>  static void generic_done(int result)
>  {
> -	command_status = result;
> +	WRITE_ONCE(command_status, result);
>  	cont = &wakeup_cont;
>  }
>  
> @@ -2873,7 +2877,7 @@ static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	list_add_tail(&bd->rq->queuelist, &floppy_reqs);
>  	spin_unlock_irq(&floppy_lock);
>  
> -	command_status = FD_COMMAND_NONE;
> +	WRITE_ONCE(command_status, FD_COMMAND_NONE);
>  	__reschedule_timeout(MAXTIMEOUT, "fd_request");
>  	set_fdc(0);
>  	process_fd_request();


^ permalink raw reply

* [PATCH 2/2] nullblk: zoned: use lock guards
From: Ricardo H H Kojo @ 2026-04-18  4:38 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block
In-Reply-To: <20260418043837.721045-1-ricardo.kojo@ime.usp.br>

Use guard() and scoped_guard() for handling spin locks instead of manually
locking and unlocking. This prevents forgotten locks due to early exits.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
---
 drivers/block/null_blk/zoned.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 384bdce6a9b7..6f59531e9b1d 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -471,13 +471,12 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 	}
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_EMPTY:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			break;
@@ -487,7 +486,6 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_CLOSED:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			dev->nr_zones_closed--;
@@ -497,8 +495,6 @@ static blk_status_t null_open_zone(struct nullb_device *dev,
 		}
 
 		dev->nr_zones_exp_open++;
-
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	zone->cond = BLK_ZONE_COND_EXP_OPEN;
@@ -526,7 +522,7 @@ static blk_status_t null_close_zone(struct nullb_device *dev,
 	}
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_IMP_OPEN:
@@ -542,7 +538,6 @@ static blk_status_t null_close_zone(struct nullb_device *dev,
 		if (zone->wp > zone->start)
 			dev->nr_zones_closed++;
 
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	if (zone->wp == zone->start)
@@ -562,17 +557,15 @@ static blk_status_t null_finish_zone(struct nullb_device *dev,
 		return BLK_STS_IOERR;
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_FULL:
 			/* Finish operation on full is not an error */
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_OK;
 		case BLK_ZONE_COND_EMPTY:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			break;
@@ -585,13 +578,11 @@ static blk_status_t null_finish_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_CLOSED:
 			ret = null_check_zone_resources(dev, zone);
 			if (ret != BLK_STS_OK) {
-				spin_unlock(&dev->zone_res_lock);
 				return ret;
 			}
 			dev->nr_zones_closed--;
 			break;
 		default:
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_IOERR;
 		}
 
@@ -611,7 +602,7 @@ static blk_status_t null_reset_zone(struct nullb_device *dev,
 		return BLK_STS_IOERR;
 
 	if (dev->need_zone_res_mgmt) {
-		spin_lock(&dev->zone_res_lock);
+		guard(spinlock)(&dev->zone_res_lock);
 
 		switch (zone->cond) {
 		case BLK_ZONE_COND_IMP_OPEN:
@@ -627,11 +618,9 @@ static blk_status_t null_reset_zone(struct nullb_device *dev,
 		case BLK_ZONE_COND_FULL:
 			break;
 		default:
-			spin_unlock(&dev->zone_res_lock);
 			return BLK_STS_IOERR;
 		}
 
-		spin_unlock(&dev->zone_res_lock);
 	}
 
 	zone->cond = BLK_ZONE_COND_EMPTY;
-- 
2.34.1


^ permalink raw reply related

* [PATCH 1/2] nullblk: main: use lock guards
From: Ricardo H H Kojo @ 2026-04-18  4:38 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block
In-Reply-To: <20260418043837.721045-1-ricardo.kojo@ime.usp.br>

Use guard() and scoped_guard() for handling mutex and spin locks instead of
manually locking and unlocking. This prevents forgotten locks due to early
exits and remove the need of gotos.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
---
 drivers/block/null_blk/main.c     | 74 +++++++++++++------------------
 drivers/block/null_blk/null_blk.h |  1 +
 2 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..fbe34e8a6c93 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -423,9 +423,8 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
 {
 	int ret;
 
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
-	mutex_unlock(&lock);
 
 	return ret;
 }
@@ -435,9 +434,8 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
 {
 	int ret;
 
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
-	mutex_unlock(&lock);
 
 	return ret;
 }
@@ -493,15 +491,15 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 		return ret;
 
 	ret = count;
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	if (!dev->power && newp) {
 		if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))
-			goto out;
+			return ret;
 
 		ret = null_add_dev(dev);
 		if (ret) {
 			clear_bit(NULLB_DEV_FL_UP, &dev->flags);
-			goto out;
+			return ret;
 		}
 
 		set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
@@ -515,8 +513,6 @@ static ssize_t nullb_device_power_store(struct config_item *item,
 		clear_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
 	}
 
-out:
-	mutex_unlock(&lock);
 	return ret;
 }
 
@@ -707,10 +703,9 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item)
 	struct nullb_device *dev = to_nullb_device(item);
 
 	if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
-		mutex_lock(&lock);
+		guard(mutex)(&lock);
 		dev->power = false;
 		null_del_dev(dev->nullb);
-		mutex_unlock(&lock);
 	}
 	nullb_del_fault_config(dev);
 	config_item_put(item);
@@ -1205,7 +1200,7 @@ blk_status_t null_handle_discard(struct nullb_device *dev,
 	size_t n = nr_sectors << SECTOR_SHIFT;
 	size_t temp;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	while (n > 0) {
 		temp = min_t(size_t, n, dev->blocksize);
 		null_free_sector(nullb, sector, false);
@@ -1214,7 +1209,6 @@ blk_status_t null_handle_discard(struct nullb_device *dev,
 		sector += temp >> SECTOR_SHIFT;
 		n -= temp;
 	}
-	spin_unlock_irq(&nullb->lock);
 
 	return BLK_STS_OK;
 }
@@ -1226,7 +1220,7 @@ static blk_status_t null_handle_flush(struct nullb *nullb)
 	if (!null_cache_active(nullb))
 		return 0;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	while (true) {
 		err = null_make_cache_space(nullb,
 			nullb->dev->cache_size * 1024 * 1024);
@@ -1235,7 +1229,6 @@ static blk_status_t null_handle_flush(struct nullb *nullb)
 	}
 
 	WARN_ON(!radix_tree_empty(&nullb->dev->cache));
-	spin_unlock_irq(&nullb->lock);
 	return errno_to_blk_status(err);
 }
 
@@ -1292,7 +1285,7 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 	struct req_iterator iter;
 	struct bio_vec bvec;
 
-	spin_lock_irq(&nullb->lock);
+	guard(spinlock_irq)(&nullb->lock);
 	rq_for_each_segment(bvec, rq, iter) {
 		len = bvec.bv_len;
 		if (transferred_bytes + len > max_bytes)
@@ -1307,7 +1300,6 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 		if (transferred_bytes >= max_bytes)
 			break;
 	}
-	spin_unlock_irq(&nullb->lock);
 
 	return err;
 }
@@ -1592,11 +1584,11 @@ static int null_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
 	int nr = 0;
 	struct request *rq;
 
-	spin_lock(&nq->poll_lock);
-	list_splice_init(&nq->poll_list, &list);
-	list_for_each_entry(rq, &list, queuelist)
-		blk_mq_set_request_complete(rq);
-	spin_unlock(&nq->poll_lock);
+	scoped_guard(spinlock, &nq->poll_lock) {
+		list_splice_init(&nq->poll_list, &list);
+		list_for_each_entry(rq, &list, queuelist)
+			blk_mq_set_request_complete(rq);
+	}
 
 	while (!list_empty(&list)) {
 		struct nullb_cmd *cmd;
@@ -1624,14 +1616,12 @@ static enum blk_eh_timer_return null_timeout_rq(struct request *rq)
 	if (hctx->type == HCTX_TYPE_POLL) {
 		struct nullb_queue *nq = hctx->driver_data;
 
-		spin_lock(&nq->poll_lock);
-		/* The request may have completed meanwhile. */
-		if (blk_mq_request_completed(rq)) {
-			spin_unlock(&nq->poll_lock);
-			return BLK_EH_DONE;
+		scoped_guard(spinlock, &nq->poll_lock) {
+			/* The request may have completed meanwhile. */
+			if (blk_mq_request_completed(rq))
+				return BLK_EH_DONE;
+			list_del_init(&rq->queuelist);
 		}
-		list_del_init(&rq->queuelist);
-		spin_unlock(&nq->poll_lock);
 	}
 
 	pr_info("rq %p timed out\n", rq);
@@ -1692,9 +1682,9 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
 	blk_mq_start_request(rq);
 
 	if (is_poll) {
-		spin_lock(&nq->poll_lock);
-		list_add_tail(&rq->queuelist, &nq->poll_list);
-		spin_unlock(&nq->poll_lock);
+		scoped_guard(spinlock, &nq->poll_lock) {
+			list_add_tail(&rq->queuelist, &nq->poll_list);
+		}
 		return BLK_STS_OK;
 	}
 	if (cmd->fake_timeout)
@@ -2081,14 +2071,13 @@ static struct nullb *null_find_dev_by_name(const char *name)
 {
 	struct nullb *nullb = NULL, *nb;
 
-	mutex_lock(&lock);
+	guard(mutex)(&lock);
 	list_for_each_entry(nb, &nullb_list, list) {
 		if (strcmp(nb->disk_name, name) == 0) {
 			nullb = nb;
 			break;
 		}
 	}
-	mutex_unlock(&lock);
 
 	return nullb;
 }
@@ -2101,10 +2090,9 @@ static int null_create_dev(void)
 	dev = null_alloc_dev();
 	if (!dev)
 		return -ENOMEM;
-
-	mutex_lock(&lock);
-	ret = null_add_dev(dev);
-	mutex_unlock(&lock);
+	scoped_guard(mutex, &lock) {
+		ret = null_add_dev(dev);
+	}
 	if (ret) {
 		null_free_dev(dev);
 		return ret;
@@ -2202,12 +2190,12 @@ static void __exit null_exit(void)
 
 	unregister_blkdev(null_major, "nullb");
 
-	mutex_lock(&lock);
-	while (!list_empty(&nullb_list)) {
-		nullb = list_entry(nullb_list.next, struct nullb, list);
-		null_destroy_dev(nullb);
+	scoped_guard(mutex, &lock) {
+		while (!list_empty(&nullb_list)) {
+			nullb = list_entry(nullb_list.next, struct nullb, list);
+			null_destroy_dev(nullb);
+		}
 	}
-	mutex_unlock(&lock);
 
 	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 6c4c4bbe7dad..c2bb085d3582 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -14,6 +14,7 @@
 #include <linux/fault-inject.h>
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
+#include <linux/cleanup.h>
 
 struct nullb_cmd {
 	blk_status_t error;
-- 
2.34.1


^ permalink raw reply related

* [PATCH 0/2] nullblk: use lock guards
From: Ricardo H H Kojo @ 2026-04-18  4:38 UTC (permalink / raw)
  To: axboe; +Cc: Ricardo H H Kojo, Ellian Carlos, Gabriel B L de Oliveira,
	linux-block

Use guard() and scoped_guard() for handling mutex and spin locks instead of
manually locking and unlocking in main.c and zoned.c. This prevents forgotten
locks due to early exits and remove the need of gotos.

Signed-off-by: Ricardo H H Kojo <ricardo.kojo@ime.usp.br>
Co-developed-by: Ellian Carlos <elliancarlos@gmail.com>
Signed-off-by: Ellian Carlos <elliancarlos@gmail.com>
Co-developed-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>
Signed-off-by: Gabriel B L de Oliveira <gabrielblo@ime.usp.br>

Ricardo H H Kojo (2):
  nullblk: main: use lock guards
  nullblk: zoned: use lock guards

 drivers/block/null_blk/main.c     | 74 +++++++++++++------------------
 drivers/block/null_blk/null_blk.h |  1 +
 drivers/block/null_blk/zoned.c    | 19 ++------
 3 files changed, 36 insertions(+), 58 deletions(-)

-- 
2.34.1


^ permalink raw reply

* Re: [PATCH 4/4] drbd: switch from genl_magic macros to YNL-generated code
From: kernel test robot @ 2026-04-18  4:36 UTC (permalink / raw)
  To: Christoph Böhmwalder, Jens Axboe
  Cc: oe-kbuild-all, drbd-dev, linux-kernel, Lars Ellenberg,
	Philipp Reisner, linux-block, Donald Hunter, Eric Dumazet,
	Jakub Kicinski, netdev, Christoph Böhmwalder
In-Reply-To: <20260407173356.873887-5-christoph.boehmwalder@linbit.com>

Hi Christoph,

kernel test robot noticed the following build errors:

[auto build test ERROR on a9c4b1d37622ed01b75f94a4f68cf55f33153a31]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-B-hmwalder/drbd-move-UAPI-headers-to-include-uapi-linux/20260417-214347
base:   a9c4b1d37622ed01b75f94a4f68cf55f33153a31
patch link:    https://lore.kernel.org/r/20260407173356.873887-5-christoph.boehmwalder%40linbit.com
patch subject: [PATCH 4/4] drbd: switch from genl_magic macros to YNL-generated code
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20260418/202604180607.iqIlyAER-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260418/202604180607.iqIlyAER-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604180607.iqIlyAER-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from <command-line>:
>> ./usr/include/linux/drbd.h:18:10: fatal error: sys/types.h: No such file or directory
      18 | #include <sys/types.h>
         |          ^~~~~~~~~~~~~
   compilation terminated.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v3 2/2] blk-mq: expose tag starvation counts via debugfs
From: Aaron Tomlin @ 2026-04-18  2:29 UTC (permalink / raw)
  To: Jens Axboe, bvanassche
  Cc: rostedt, mhiramat, mathieu.desnoyers, johannes.thumshirn, kch,
	dlemoal, ritesh.list, loberman, neelx, sean, mproche, chjohnst,
	linux-block, linux-kernel, linux-trace-kernel
In-Reply-To: <424110a4-4e14-4e40-af0d-59ca7a30d904@kernel.dk>

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

On Fri, Apr 17, 2026 at 02:16:07PM -0600, Jens Axboe wrote:
> On 4/17/26 12:28 PM, Bart Van Assche wrote:
> > On 3/19/26 3:19 PM, Aaron Tomlin wrote:
> >> To guarantee zero performance overhead for production kernels compiled
> >> without debugfs, the underlying atomic_t variables and their associated
> >> increment routines are strictly guarded behind CONFIG_BLK_DEBUG_FS.
> >> When this configuration is disabled, the tracking logic compiles down
> >> to a safe no-op.
> > 
> > I don't think that's sufficient. Please use per-cpu counters to
> > minimize the overhead for kernels in which debugfs is enabled.
> 
> Agree, this is the usual nonsense of thinking you can hide any overhead
> behind a config option, when in practice production kernels very much DO
> have CONFIG_DEBUGFS enabled.

Hi Bart, Jens,

Thank you both for the candid feedback.

I concede that relying on CONFIG_BLK_DEBUG_FS to mitigate the performance
impact was a short-sighted approach, as you are absolutely right that
debugfs is routinely enabled in modern production environments. The use of
an atomic_t variable in this case would indeed introduce unacceptable cache
line contention under heavy workloads.

I shall address this in the next iteration by transitioning the tracking
logic entirely to per-CPU counters. This will ensure the overhead is
strictly minimised, irrespective of the build configuration.

Thank you again for your time and guidance on this matter.


Kind regards,
-- 
Aaron Tomlin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox