linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Increase SCSI IOPS
@ 2025-11-17 22:51 Bart Van Assche
  2025-11-21 21:17 ` Bart Van Assche
  0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-11-17 22:51 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche

Hi Martin,

This patch series increases scsi_debug IOPS by 5% on my test setup by disabling
SCSI budget management if it is not needed.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Changes compared to v1 (https://lore.kernel.org/linux-scsi/20250910213254.1215318-1-bvanassche@acm.org/):
 - Added three block layer patches to introduce the function
   blk_mq_tagset_iter().
 - Applied the optimization not only for host-wide tags but also if there is
   only a single hardware queue.
 - Renamed scsi_device_check_in_flight() into scsi_device_check_allocated().
 - Added support for set->shared_tags == NULL in scsi_device_busy().

Bart Van Assche (5):
  block: Rename busy_tag_iter_fn into blk_mq_rq_iter_fn
  block: Introduce __blk_mq_tagset_iter()
  block: Introduce blk_mq_tagset_iter()
  scsi: core: Generalize scsi_device_busy()
  scsi: core: Improve IOPS in case of host-wide tags

 block/blk-mq-tag.c         | 67 ++++++++++++++++++++++++++------------
 block/blk-mq.h             |  4 +--
 drivers/scsi/scsi.c        |  3 +-
 drivers/scsi/scsi_lib.c    | 38 +++++++++++++++++++++
 drivers/scsi/scsi_scan.c   | 18 +++++++++-
 include/linux/blk-mq.h     |  6 ++--
 include/scsi/scsi_device.h |  5 +--
 7 files changed, 110 insertions(+), 31 deletions(-)


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

* Re: [PATCH v2 0/5] Increase SCSI IOPS
  2025-11-17 22:51 Bart Van Assche
@ 2025-11-21 21:17 ` Bart Van Assche
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-21 21:17 UTC (permalink / raw)
  To: Martin K . Petersen; +Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke

On 11/17/25 2:51 PM, Bart Van Assche wrote:
> This patch series increases scsi_debug IOPS by 5% on my test setup by disabling
> SCSI budget management if it is not needed.
(replying to my own email)

The kernel test robot reported that this patch series introduces a hang
during LUN scanning for ATA devices. I have been able to reproduce this
hang in a VM. The root cause has been identified and a fix is under
test. I will post a new version of this patch series after testing has
finished.

Thanks,

Bart.

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

* [PATCH v2 0/5] Increase SCSI IOPS
@ 2025-11-24 18:21 Bart Van Assche
  2025-11-24 18:21 ` [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter() Bart Van Assche
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:21 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche

Hi Martin,

This patch series increases scsi_debug IOPS by 5% on my test setup by disabling
SCSI budget management if it is not needed. This patch series improves the
performance of many SCSI LLDs, including the UFS and ATA drivers.

Please consider this patch series for the next merge window.

Thanks,

Bart.

Changes compared to v1:
 - Fixed a hang during LUN scanning for ATA devices.

Bart Van Assche (5):
  block: Introduce __blk_mq_tagset_iter()
  block: Introduce blk_mq_tagset_iter()
  libata: Stop using cmd->budget_token
  scsi: core: Generalize scsi_device_busy()
  scsi: core: Improve IOPS in case of host-wide tags

 block/blk-mq-tag.c         | 51 ++++++++++++++++++++++++++++----------
 drivers/ata/libata-scsi.c  | 18 +++++---------
 drivers/scsi/scsi.c        |  6 ++---
 drivers/scsi/scsi_lib.c    | 38 ++++++++++++++++++++++++++++
 drivers/scsi/scsi_scan.c   | 18 +++++++++++++-
 include/linux/blk-mq.h     |  2 ++
 include/scsi/scsi_device.h |  5 +---
 7 files changed, 104 insertions(+), 34 deletions(-)


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

* [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter()
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
@ 2025-11-24 18:21 ` Bart Van Assche
  2025-11-25  6:32   ` Christoph Hellwig
  2025-11-24 18:21 ` [PATCH v2 2/5] block: Introduce blk_mq_tagset_iter() Bart Van Assche
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:21 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche, Jens Axboe, Christoph Hellwig, Ming Lei

Prepare for introducing a second caller of __blk_mq_tagset_iter(). No
functionality has been changed.

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq-tag.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 8a61c481015e..f169beeded64 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -419,6 +419,24 @@ void blk_mq_all_tag_iter(struct blk_mq_tags *tags, blk_mq_rq_iter_fn *fn,
 	__blk_mq_all_tag_iter(tags, fn, priv, BT_TAG_ITER_STATIC_RQS);
 }
 
+static void __blk_mq_tagset_iter(struct blk_mq_tag_set *tagset,
+			blk_mq_rq_iter_fn *fn, void *priv, unsigned long flags)
+{
+	int i, nr_tags, srcu_idx;
+
+	srcu_idx = srcu_read_lock(&tagset->tags_srcu);
+
+	nr_tags = blk_mq_is_shared_tags(tagset->flags) ? 1 :
+		tagset->nr_hw_queues;
+
+	for (i = 0; i < nr_tags; i++) {
+		if (tagset->tags && tagset->tags[i])
+			__blk_mq_all_tag_iter(tagset->tags[i], fn, priv,
+					      flags);
+	}
+	srcu_read_unlock(&tagset->tags_srcu, srcu_idx);
+}
+
 /**
  * blk_mq_tagset_busy_iter - iterate over all started requests in a tag set
  * @tagset:	Tag set to iterate over.
@@ -434,19 +452,7 @@ void blk_mq_all_tag_iter(struct blk_mq_tags *tags, blk_mq_rq_iter_fn *fn,
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		blk_mq_rq_iter_fn *fn, void *priv)
 {
-	unsigned int flags = tagset->flags;
-	int i, nr_tags, srcu_idx;
-
-	srcu_idx = srcu_read_lock(&tagset->tags_srcu);
-
-	nr_tags = blk_mq_is_shared_tags(flags) ? 1 : tagset->nr_hw_queues;
-
-	for (i = 0; i < nr_tags; i++) {
-		if (tagset->tags && tagset->tags[i])
-			__blk_mq_all_tag_iter(tagset->tags[i], fn, priv,
-					      BT_TAG_ITER_STARTED);
-	}
-	srcu_read_unlock(&tagset->tags_srcu, srcu_idx);
+	__blk_mq_tagset_iter(tagset, fn, priv, BT_TAG_ITER_STARTED);
 }
 EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
 

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

* [PATCH v2 2/5] block: Introduce blk_mq_tagset_iter()
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
  2025-11-24 18:21 ` [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter() Bart Van Assche
@ 2025-11-24 18:21 ` Bart Van Assche
  2025-11-24 18:21 ` [PATCH v2 3/5] libata: Stop using cmd->budget_token Bart Van Assche
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:21 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche, Jens Axboe, Christoph Hellwig, Ming Lei

Support iterating over all requests in a tag set, including requests
that have not yet been started. A later patch will call this function
from scsi_device_busy().

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq-tag.c     | 19 +++++++++++++++++++
 include/linux/blk-mq.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index f169beeded64..f277ed7e7743 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -456,6 +456,25 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 }
 EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
 
+/**
+ * blk_mq_tagset_iter - iterate over all requests in a tag set
+ * @tagset:	Tag set to iterate over.
+ * @fn:		Pointer to the function that will be called for each request.
+ *		@fn will be called as follows: @fn(rq, @priv) where rq is a
+ *		pointer to a request. Return true to continue iterating tags,
+ *		false to stop.
+ * @priv:	Will be passed as second argument to @fn.
+ *
+ * We grab one request reference before calling @fn and release it after
+ * @fn returns.
+ */
+void blk_mq_tagset_iter(struct blk_mq_tag_set *tagset, blk_mq_rq_iter_fn *fn,
+			void *priv)
+{
+	__blk_mq_tagset_iter(tagset, fn, priv, 0);
+}
+EXPORT_SYMBOL(blk_mq_tagset_iter);
+
 static bool blk_mq_tagset_count_completed_rqs(struct request *rq, void *data)
 {
 	unsigned *count = data;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 3467cacb281c..20a22c1cd067 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -927,6 +927,8 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs);
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		blk_mq_rq_iter_fn *fn, void *priv);
+void blk_mq_tagset_iter(struct blk_mq_tag_set *tagset, blk_mq_rq_iter_fn *fn,
+		void *priv);
 void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset);
 void blk_mq_freeze_queue_nomemsave(struct request_queue *q);
 void blk_mq_unfreeze_queue_nomemrestore(struct request_queue *q);

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

* [PATCH v2 3/5] libata: Stop using cmd->budget_token
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
  2025-11-24 18:21 ` [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter() Bart Van Assche
  2025-11-24 18:21 ` [PATCH v2 2/5] block: Introduce blk_mq_tagset_iter() Bart Van Assche
@ 2025-11-24 18:21 ` Bart Van Assche
  2025-11-25  4:35   ` Damien Le Moal
  2025-11-24 18:21 ` [PATCH v2 4/5] scsi: core: Generalize scsi_device_busy() Bart Van Assche
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:21 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche, Damien Le Moal, Niklas Cassel, Christoph Hellwig

Since a single hardware queue is used by ATA drivers, the request tag
uniquely identifies in-flight commands. Stop using the SCSI budget token
to prepare for no longer allocating a budget token if possible. The
modified code was introduced by commit 4f1a22ee7b57 ("libata: Improve ATA
queued command allocation").

Cc: Damien Le Moal <dlemoal@kernel.org>
Cc: Niklas Cassel <cassel@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ata/libata-scsi.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 026122bb6f2f..90f5422fa08b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -744,22 +744,16 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
 {
 	struct ata_port *ap = dev->link->ap;
 	struct ata_queued_cmd *qc;
-	int tag;
+	int tag = scsi_cmd_to_rq(cmd)->tag;
 
 	if (unlikely(ata_port_is_frozen(ap)))
 		goto fail;
 
-	if (ap->flags & ATA_FLAG_SAS_HOST) {
-		/*
-		 * SAS hosts may queue > ATA_MAX_QUEUE commands so use
-		 * unique per-device budget token as a tag.
-		 */
-		if (WARN_ON_ONCE(cmd->budget_token >= ATA_MAX_QUEUE))
-			goto fail;
-		tag = cmd->budget_token;
-	} else {
-		tag = scsi_cmd_to_rq(cmd)->tag;
-	}
+	WARN_ON_ONCE(cmd->device->host->tag_set.nr_hw_queues > 1);
+
+	/* SAS hosts may queue > ATA_MAX_QUEUE commands. */
+	if (ap->flags & ATA_FLAG_SAS_HOST && WARN_ON_ONCE(tag >= ATA_MAX_QUEUE))
+		goto fail;
 
 	qc = __ata_qc_from_tag(ap, tag);
 	qc->tag = qc->hw_tag = tag;

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

* [PATCH v2 4/5] scsi: core: Generalize scsi_device_busy()
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
                   ` (2 preceding siblings ...)
  2025-11-24 18:21 ` [PATCH v2 3/5] libata: Stop using cmd->budget_token Bart Van Assche
@ 2025-11-24 18:21 ` Bart Van Assche
  2025-11-24 18:22 ` [PATCH v2 5/5] scsi: core: Improve IOPS in case of host-wide tags Bart Van Assche
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:21 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche, Jens Axboe, Christoph Hellwig, Ming Lei,
	James E.J. Bottomley

Instead of only handling dev->budget_map.map != NULL, also handle
dev->budget_map.map == NULL. This patch prepares for supporting logical
units without budget map (sdev->budget_map.map == NULL).

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c    | 38 ++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |  5 +----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 51ad2ad07e43..ddc51472b5eb 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -446,6 +446,44 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
 
+struct sdev_cmds_allocated_data {
+	const struct scsi_device *sdev;
+	int count;
+};
+
+static bool scsi_device_check_allocated(struct request *rq, void *data)
+{
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
+	struct sdev_cmds_allocated_data *sifd = data;
+
+	if (cmd->device == sifd->sdev)
+		sifd->count++;
+
+	return true;
+}
+
+/**
+ * scsi_device_busy() - Number of commands allocated for a SCSI device
+ * @sdev: SCSI device.
+ *
+ * Note: There is a subtle difference between this function and
+ * scsi_host_busy(). scsi_host_busy() counts the number of commands that have
+ * been started. This function counts the number of commands that have been
+ * allocated. At least the UFS driver depends on this function counting commands
+ * that have already been allocated but that have not yet been started.
+ */
+int scsi_device_busy(const struct scsi_device *sdev)
+{
+	struct sdev_cmds_allocated_data sifd = { .sdev = sdev };
+	struct blk_mq_tag_set *set = &sdev->host->tag_set;
+
+	if (sdev->budget_map.map)
+		return sbitmap_weight(&sdev->budget_map);
+	blk_mq_tagset_iter(set, scsi_device_check_allocated, &sifd);
+	return sifd.count;
+}
+EXPORT_SYMBOL(scsi_device_busy);
+
 static inline bool scsi_device_is_busy(struct scsi_device *sdev)
 {
 	if (scsi_device_busy(sdev) >= sdev->queue_depth)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d32f5841f4f8..0dd078ac9b89 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -713,10 +713,7 @@ static inline int scsi_device_supports_vpd(struct scsi_device *sdev)
 	return 0;
 }
 
-static inline int scsi_device_busy(struct scsi_device *sdev)
-{
-	return sbitmap_weight(&sdev->budget_map);
-}
+int scsi_device_busy(const struct scsi_device *sdev);
 
 /* Macros to access the UNIT ATTENTION counters */
 #define scsi_get_ua_new_media_ctr(sdev)	atomic_read(&sdev->ua_new_media_ctr)

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

* [PATCH v2 5/5] scsi: core: Improve IOPS in case of host-wide tags
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
                   ` (3 preceding siblings ...)
  2025-11-24 18:21 ` [PATCH v2 4/5] scsi: core: Generalize scsi_device_busy() Bart Van Assche
@ 2025-11-24 18:22 ` Bart Van Assche
  2025-11-25  8:20 ` [PATCH v2 0/5] Increase SCSI IOPS Niklas Cassel
  2025-11-25  9:08 ` John Garry
  6 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-24 18:22 UTC (permalink / raw)
  To: Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Bart Van Assche, Jens Axboe, Christoph Hellwig, Ming Lei,
	James E.J. Bottomley

The SCSI core uses the budget map to restrict the number of commands
that are in flight per logical unit. That limit check can be left out if
host->cmd_per_lun >= host->can_queue and if the host tag set is shared
across all hardware queues or if there is only one hardware queue  Since
scsi_mq_get_budget() shows up in all CPU profiles for fast SCSI devices,
do not allocate a budget map if cmd_per_lun >= can_queue and if the host
tag set is shared across all hardware queues.

For the following test this patch increases IOPS by 5%:

modprobe scsi_debug delay=0 no_rwlock=1 host_max_queue=192 submit_queues=$(nproc)

fio --bs=4096 --disable_clat=1 --disable_slat=1 --group_reporting=1 \
  --gtod_reduce=1 --invalidate=1 --ioengine=io_uring --ioscheduler=none \
  --norandommap --runtime=60 --rw=randread --thread --time_based=1 \
  --buffered=0 --numjobs=1 --iodepth=192 --iodepth_batch=24 --name=/dev/sda \
  --filename=/dev/sda

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi.c      |  6 ++----
 drivers/scsi/scsi_scan.c | 18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 76cdad063f7b..3dc93dd9fda2 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -216,9 +216,6 @@ int scsi_device_max_queue_depth(struct scsi_device *sdev)
  */
 int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
 {
-	if (!sdev->budget_map.map)
-		return -EINVAL;
-
 	depth = min_t(int, depth, scsi_device_max_queue_depth(sdev));
 
 	if (depth > 0) {
@@ -229,7 +226,8 @@ int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
 	if (sdev->request_queue)
 		blk_set_queue_depth(sdev->request_queue, depth);
 
-	sbitmap_resize(&sdev->budget_map, sdev->queue_depth);
+	if (sdev->budget_map.map)
+		sbitmap_resize(&sdev->budget_map, sdev->queue_depth);
 
 	return sdev->queue_depth;
 }
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 7acbfcfc2172..99b82e28f292 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -215,9 +215,17 @@ static void scsi_unlock_floptical(struct scsi_device *sdev,
 			 SCSI_TIMEOUT, 3, NULL);
 }
 
+static bool scsi_needs_budget_map(struct Scsi_Host *shost, unsigned int depth)
+{
+	if (shost->host_tagset || shost->tag_set.nr_hw_queues == 1)
+		return depth < shost->can_queue;
+	return true;
+}
+
 static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
 					unsigned int depth)
 {
+	struct Scsi_Host *shost = sdev->host;
 	int new_shift = sbitmap_calculate_shift(depth);
 	bool need_alloc = !sdev->budget_map.map;
 	bool need_free = false;
@@ -225,6 +233,13 @@ static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
 	int ret;
 	struct sbitmap sb_backup;
 
+	if (!scsi_needs_budget_map(shost, depth)) {
+		memflags = blk_mq_freeze_queue(sdev->request_queue);
+		sbitmap_free(&sdev->budget_map);
+		blk_mq_unfreeze_queue(sdev->request_queue, memflags);
+		return 0;
+	}
+
 	depth = min_t(unsigned int, depth, scsi_device_max_queue_depth(sdev));
 
 	/*
@@ -1120,7 +1135,8 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	scsi_cdl_check(sdev);
 
 	sdev->max_queue_depth = sdev->queue_depth;
-	WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
+	WARN_ON_ONCE(sdev->budget_map.map &&
+		     sdev->max_queue_depth > sdev->budget_map.depth);
 
 	/*
 	 * Ok, the device is now all set up, we can

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

* Re: [PATCH v2 3/5] libata: Stop using cmd->budget_token
  2025-11-24 18:21 ` [PATCH v2 3/5] libata: Stop using cmd->budget_token Bart Van Assche
@ 2025-11-25  4:35   ` Damien Le Moal
  2025-11-25 17:14     ` Bart Van Assche
  0 siblings, 1 reply; 16+ messages in thread
From: Damien Le Moal @ 2025-11-25  4:35 UTC (permalink / raw)
  To: Bart Van Assche, Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Niklas Cassel, Christoph Hellwig

On 11/25/25 3:21 AM, Bart Van Assche wrote:
> Since a single hardware queue is used by ATA drivers, the request tag
> uniquely identifies in-flight commands. Stop using the SCSI budget token
> to prepare for no longer allocating a budget token if possible. The
> modified code was introduced by commit 4f1a22ee7b57 ("libata: Improve ATA
> queued command allocation").
> 
> Cc: Damien Le Moal <dlemoal@kernel.org>
> Cc: Niklas Cassel <cassel@kernel.org>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

It is very hard to review this without the cover letter and the other patches
to give context.

So as-is, without trying to find where the other patches are, this is a big no
from me: this will break drivers for SAS HBAs that use libsas, and so libata as
their SAT implementation. In this case, the tag of a scsi command request is
the HBA tag, NOT the device tag. So this simply does not work at all.

Maybe you fixed that in other patches of this series. I don't know. If you want
a proper review with context, please send everything to the people from whom
you expect a review.

-- 
Damien Le Moal
Western Digital Research

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

* Re: [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter()
  2025-11-24 18:21 ` [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter() Bart Van Assche
@ 2025-11-25  6:32   ` Christoph Hellwig
  2025-11-25 16:58     ` Bart Van Assche
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2025-11-25  6:32 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, linux-scsi, linux-block, John Garry,
	Hannes Reinecke, Jens Axboe, Christoph Hellwig, Ming Lei

Why are you Ccing me on what appears 4 out of 5 patches, missing one
and the cover letter?


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

* Re: [PATCH v2 0/5] Increase SCSI IOPS
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
                   ` (4 preceding siblings ...)
  2025-11-24 18:22 ` [PATCH v2 5/5] scsi: core: Improve IOPS in case of host-wide tags Bart Van Assche
@ 2025-11-25  8:20 ` Niklas Cassel
  2025-11-25 16:45   ` Bart Van Assche
  2025-11-25  9:08 ` John Garry
  6 siblings, 1 reply; 16+ messages in thread
From: Niklas Cassel @ 2025-11-25  8:20 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Martin K . Petersen, linux-scsi, linux-block, John Garry,
	Hannes Reinecke

On Mon, Nov 24, 2025 at 10:21:55AM -0800, Bart Van Assche wrote:
> Hi Martin,
> 
> This patch series increases scsi_debug IOPS by 5% on my test setup by disabling
> SCSI budget management if it is not needed. This patch series improves the
> performance of many SCSI LLDs, including the UFS and ATA drivers.
> 
> Please consider this patch series for the next merge window.

Hello Bart,

The subject is:
[PATCH v2 0/5] Increase SCSI IOPS

AFAICT, you already sent a v2 series a few days ago:
https://lore.kernel.org/linux-scsi/20251117225205.2024479-1-bvanassche@acm.org/

I assume that you simply forgot to increase the version count.

If you respin, perhaps label it as v4, to make things less confusing.


Kind regards,
Niklas

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

* Re: [PATCH v2 0/5] Increase SCSI IOPS
  2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
                   ` (5 preceding siblings ...)
  2025-11-25  8:20 ` [PATCH v2 0/5] Increase SCSI IOPS Niklas Cassel
@ 2025-11-25  9:08 ` John Garry
  2025-11-25 16:51   ` Bart Van Assche
  6 siblings, 1 reply; 16+ messages in thread
From: John Garry @ 2025-11-25  9:08 UTC (permalink / raw)
  To: Bart Van Assche, Martin K . Petersen
  Cc: linux-scsi, linux-block, Hannes Reinecke, hch, dlemoal, cassel

On 24/11/2025 18:21, Bart Van Assche wrote:
> Hi Martin,
> 
> This patch series increases scsi_debug IOPS by 5% on my test setup by disabling
> SCSI budget management if it is not needed.

Performance results from scsi_debug are not a real acid test.

> This patch series improves the
> performance of many SCSI LLDs, including the UFS and ATA drivers.

Please provide results from real HW / real scenarios.

> 
> Please consider this patch series for the next merge window.
> 
> Thanks,
> 
> Bart.
> 
> Changes compared to v1:
>   - Fixed a hang during LUN scanning for ATA devices.
> 
> Bart Van Assche (5):
>    block: Introduce __blk_mq_tagset_iter()
>    block: Introduce blk_mq_tagset_iter()
>    libata: Stop using cmd->budget_token
>    scsi: core: Generalize scsi_device_busy()
>    scsi: core: Improve IOPS in case of host-wide tags
> 
>   block/blk-mq-tag.c         | 51 ++++++++++++++++++++++++++++----------
>   drivers/ata/libata-scsi.c  | 18 +++++---------
>   drivers/scsi/scsi.c        |  6 ++---
>   drivers/scsi/scsi_lib.c    | 38 ++++++++++++++++++++++++++++
>   drivers/scsi/scsi_scan.c   | 18 +++++++++++++-
>   include/linux/blk-mq.h     |  2 ++
>   include/scsi/scsi_device.h |  5 +---
>   7 files changed, 104 insertions(+), 34 deletions(-)
> 


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

* Re: [PATCH v2 0/5] Increase SCSI IOPS
  2025-11-25  8:20 ` [PATCH v2 0/5] Increase SCSI IOPS Niklas Cassel
@ 2025-11-25 16:45   ` Bart Van Assche
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-25 16:45 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Martin K . Petersen, linux-scsi, linux-block, John Garry,
	Hannes Reinecke

On 11/25/25 1:20 AM, Niklas Cassel wrote:
> The subject is:
> [PATCH v2 0/5] Increase SCSI IOPS
> 
> AFAICT, you already sent a v2 series a few days ago:
> https://lore.kernel.org/linux-scsi/20251117225205.2024479-1-bvanassche@acm.org/
> 
> I assume that you simply forgot to increase the version count.

Correct.
> If you respin, perhaps label it as v4, to make things less confusing.

Sure, I will do that.

Thanks,

Bart.

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

* Re: [PATCH v2 0/5] Increase SCSI IOPS
  2025-11-25  9:08 ` John Garry
@ 2025-11-25 16:51   ` Bart Van Assche
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-25 16:51 UTC (permalink / raw)
  To: John Garry, Martin K . Petersen
  Cc: linux-scsi, linux-block, Hannes Reinecke, hch, dlemoal, cassel

On 11/25/25 2:08 AM, John Garry wrote:
> Please provide results from real HW / real scenarios.

These have already been provided before. From
https://lore.kernel.org/linux-scsi/20250910213254.1215318-4-bvanassche@acm.org/:
"On my UFS 4 test setup this patch improves IOPS by 1% and reduces the
time spent in scsi_mq_get_budget() from 0.22% to 0.01%." In that test
I/O was submitted from a single CPU core.

UFS 5 devices are expected to become available soon (2026). UFS 5
devices are expected to support up to one million IOPS. That is the
double of what the fastest UFS 4 devices support. Hence my interest in
improving performance of the SCSI core.

Thanks,

Bart.

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

* Re: [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter()
  2025-11-25  6:32   ` Christoph Hellwig
@ 2025-11-25 16:58     ` Bart Van Assche
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-25 16:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Martin K . Petersen, linux-scsi, linux-block, John Garry,
	Hannes Reinecke, Jens Axboe, Ming Lei

On 11/24/25 11:32 PM, Christoph Hellwig wrote:
> Why are you Ccing me on what appears 4 out of 5 patches, missing one
> and the cover letter?
Hi Christoph,

This patch series will have to be resent. When I resend it I will Cc you
on the entire patch series including the cover letter. If you would like
to take a look now at the cover letter and the other patches in this
series, these are available here:
https://lore.kernel.org/linux-scsi/20251124182201.737160-1-bvanassche@acm.org/

Thanks,

Bart.

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

* Re: [PATCH v2 3/5] libata: Stop using cmd->budget_token
  2025-11-25  4:35   ` Damien Le Moal
@ 2025-11-25 17:14     ` Bart Van Assche
  0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-11-25 17:14 UTC (permalink / raw)
  To: Damien Le Moal, Martin K . Petersen
  Cc: linux-scsi, linux-block, John Garry, Hannes Reinecke,
	Niklas Cassel, Christoph Hellwig

On 11/24/25 9:35 PM, Damien Le Moal wrote:
> On 11/25/25 3:21 AM, Bart Van Assche wrote:
>> Since a single hardware queue is used by ATA drivers, the request tag
>> uniquely identifies in-flight commands. Stop using the SCSI budget token
>> to prepare for no longer allocating a budget token if possible. The
>> modified code was introduced by commit 4f1a22ee7b57 ("libata: Improve ATA
>> queued command allocation").
>>
>> Cc: Damien Le Moal <dlemoal@kernel.org>
>> Cc: Niklas Cassel <cassel@kernel.org>
>> Cc: John Garry <john.g.garry@oracle.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> 
> It is very hard to review this without the cover letter and the other patches
> to give context.
> 
> So as-is, without trying to find where the other patches are, this is a big no
> from me: this will break drivers for SAS HBAs that use libsas, and so libata as
> their SAT implementation. In this case, the tag of a scsi command request is
> the HBA tag, NOT the device tag. So this simply does not work at all.
> 
> Maybe you fixed that in other patches of this series. I don't know. If you want
> a proper review with context, please send everything to the people from whom
> you expect a review.

Hi Damien,

Thank you for having provided feedback on this patch.

This patch series will have to be resent. When I resend it I will Cc you
on the entire patch series including the cover letter. If you would like
to take a look now at the cover letter and the other patches in this
series, these are available here:
https://lore.kernel.org/linux-scsi/20251124182201.737160-1-bvanassche@acm.org/

How about excluding ATA and SAS kernel drivers from this patch series by
dropping this patch and by applying something like the patch below on
top of this patch series?

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
@@ -4493,6 +4499,7 @@ int ata_scsi_add_hosts(struct ata_host *host, 
const struct scsi_host_template *s
  		shost->max_lun = 1;
  		shost->max_channel = 1;
  		shost->max_cmd_len = 32;
+		shost->needs_budget_token = true;

  		/* Schedule policy is determined by ->qc_defer()
  		 * callback and it needs to see every deferred qc.
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
@@ -217,6 +217,8 @@ static void scsi_unlock_floptical(struct scsi_device 
*sdev,

  static bool scsi_needs_budget_map(struct Scsi_Host *shost, unsigned 
int depth)
  {
+	if (shost->needs_budget_token)
+		return true;
  	if (shost->host_tagset || shost->tag_set.nr_hw_queues == 1)
  		return depth < shost->can_queue;
  	return true;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index e87cf7eadd26..2b3fc8dcbf0b 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -695,6 +695,9 @@ struct Scsi_Host {
  	/* The transport requires the LUN bits NOT to be stored in CDB[1] */
  	unsigned no_scsi2_lun_in_cdb:1;

+	/* Whether the LLD uses cmd->budget_token */
+	unsigned needs_budget_token:1;
+
  	/*
  	 * Optional work queue to be utilized by the transport
  	 */

Thanks,

Bart.

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

end of thread, other threads:[~2025-11-25 17:15 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-24 18:21 [PATCH v2 0/5] Increase SCSI IOPS Bart Van Assche
2025-11-24 18:21 ` [PATCH v2 1/5] block: Introduce __blk_mq_tagset_iter() Bart Van Assche
2025-11-25  6:32   ` Christoph Hellwig
2025-11-25 16:58     ` Bart Van Assche
2025-11-24 18:21 ` [PATCH v2 2/5] block: Introduce blk_mq_tagset_iter() Bart Van Assche
2025-11-24 18:21 ` [PATCH v2 3/5] libata: Stop using cmd->budget_token Bart Van Assche
2025-11-25  4:35   ` Damien Le Moal
2025-11-25 17:14     ` Bart Van Assche
2025-11-24 18:21 ` [PATCH v2 4/5] scsi: core: Generalize scsi_device_busy() Bart Van Assche
2025-11-24 18:22 ` [PATCH v2 5/5] scsi: core: Improve IOPS in case of host-wide tags Bart Van Assche
2025-11-25  8:20 ` [PATCH v2 0/5] Increase SCSI IOPS Niklas Cassel
2025-11-25 16:45   ` Bart Van Assche
2025-11-25  9:08 ` John Garry
2025-11-25 16:51   ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2025-11-17 22:51 Bart Van Assche
2025-11-21 21:17 ` Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).