public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Douglas Gilbert <dgilbert@interlog.com>
To: linux-scsi@vger.kernel.org
Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
	kashyap.desai@broadcom.com, ming.lei@redhat.com,
	john.garry@huawei.com, axboe@kernel.dk
Subject: [PATCH] scsi_debug: fix cmd_per_lun, set to max_queue
Date: Wed, 14 Apr 2021 21:50:31 -0400	[thread overview]
Message-ID: <20210415015031.607153-1-dgilbert@interlog.com> (raw)

Make sure that the cmd_per_lun value placed in the host template
never exceeds the can_queue value. If the max_queue driver
parameter is not specified then both cmd_per_lun and can_queue are
set to CAN_QUEUE. CAN_QUEUE is a compile time constant and is used
to dimension an array to hold queued requests. If the max_queue
driver parameter is given it is must be less than or equal to
CAN_QUEUE and if so, the host template values are adjusted.

Remove undocumented code that allowed queue_depth to exceed
CAN_QUEUE and cause stack full type errors. There is a documented
way to do that with every_nth and
    echo 0x8000 > /sys/bus/pseudo/drivers/scsi_debug/opts
See: https://sg.danny.cz/sg/scsi_debug.html

Tweak some formatting, and add a suggestion to the "trim
poll_queues" warning.

Reported-by: Kashyap Desai <kashyap.desai@broadcom.com>
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/scsi_debug.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 70165be10f00..a5d1633b5bd8 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -218,7 +218,7 @@ static const char *sdebug_version_date = "20200710";
  */
 #define SDEBUG_CANQUEUE_WORDS  3	/* a WORD is bits in a long */
 #define SDEBUG_CANQUEUE  (SDEBUG_CANQUEUE_WORDS * BITS_PER_LONG)
-#define DEF_CMD_PER_LUN  255
+#define DEF_CMD_PER_LUN  SDEBUG_CANQUEUE
 
 /* UA - Unit Attention; SA - Service Action; SSU - Start Stop Unit */
 #define F_D_IN			1	/* Data-in command (e.g. READ) */
@@ -5695,8 +5695,8 @@ MODULE_PARM_DESC(lbpu, "enable LBP, support UNMAP command (def=0)");
 MODULE_PARM_DESC(lbpws, "enable LBP, support WRITE SAME(16) with UNMAP bit (def=0)");
 MODULE_PARM_DESC(lbpws10, "enable LBP, support WRITE SAME(10) with UNMAP bit (def=0)");
 MODULE_PARM_DESC(lowest_aligned, "lowest aligned lba (def=0)");
-MODULE_PARM_DESC(max_luns, "number of LUNs per target to simulate(def=1)");
 MODULE_PARM_DESC(lun_format, "LUN format: 0->peripheral (def); 1 --> flat address method");
+MODULE_PARM_DESC(max_luns, "number of LUNs per target to simulate(def=1)");
 MODULE_PARM_DESC(max_queue, "max number of queued commands (1 to max(def))");
 MODULE_PARM_DESC(medium_error_count, "count of sectors to return follow on MEDIUM error");
 MODULE_PARM_DESC(medium_error_start, "starting sector number to return MEDIUM error");
@@ -5710,7 +5710,7 @@ MODULE_PARM_DESC(opt_xferlen_exp, "optimal transfer length granularity exponent
 MODULE_PARM_DESC(opts, "1->noise, 2->medium_err, 4->timeout, 8->recovered_err... (def=0)");
 MODULE_PARM_DESC(per_host_store, "If set, next positive add_host will get new store (def=0)");
 MODULE_PARM_DESC(physblk_exp, "physical block exponent (def=0)");
-MODULE_PARM_DESC(poll_queues, "support for iouring iopoll queues (1 to max(submit_queues - 1)");
+MODULE_PARM_DESC(poll_queues, "support for iouring iopoll queues (1 to max(submit_queues - 1))");
 MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
 MODULE_PARM_DESC(random, "If set, uniformly randomize command duration between 0 and delay_in_ns");
 MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
@@ -7165,12 +7165,15 @@ static int sdebug_change_qdepth(struct scsi_device *sdev, int qdepth)
 	}
 	num_in_q = atomic_read(&devip->num_in_q);
 
+	if (qdepth > SDEBUG_CANQUEUE) {
+		qdepth = SDEBUG_CANQUEUE;
+		pr_warn("%s: requested qdepth [%d] exceeds canqueue [%d], trim\n", __func__,
+			qdepth, SDEBUG_CANQUEUE);
+	}
 	if (qdepth < 1)
 		qdepth = 1;
-	/* allow to exceed max host qc_arr elements for testing */
-	if (qdepth > SDEBUG_CANQUEUE + 10)
-		qdepth = SDEBUG_CANQUEUE + 10;
-	scsi_change_queue_depth(sdev, qdepth);
+	if (qdepth != sdev->queue_depth)
+		scsi_change_queue_depth(sdev, qdepth);
 
 	if (SDEBUG_OPT_Q_NOISE & sdebug_opts) {
 		sdev_printk(KERN_INFO, sdev, "%s: qdepth=%d, num_in_q=%d\n",
@@ -7558,6 +7561,7 @@ static int sdebug_driver_probe(struct device *dev)
 	sdbg_host = to_sdebug_host(dev);
 
 	sdebug_driver_template.can_queue = sdebug_max_queue;
+	sdebug_driver_template.cmd_per_lun = sdebug_max_queue;
 	if (!sdebug_clustering)
 		sdebug_driver_template.dma_boundary = PAGE_SIZE - 1;
 
@@ -7593,7 +7597,11 @@ static int sdebug_driver_probe(struct device *dev)
 	 * If condition not met, trim poll_queues to 1 (just for simplicity).
 	 */
 	if (poll_queues >= submit_queues) {
-		pr_warn("%s: trim poll_queues to 1\n", my_name);
+		if (submit_queues < 3)
+			pr_warn("%s: trim poll_queues to 1\n", my_name);
+		else
+			pr_warn("%s: trim poll_queues to 1. Perhaps try poll_queues=%d\n",
+				my_name, submit_queues - 1);
 		poll_queues = 1;
 	}
 	if (poll_queues)
-- 
2.25.1


             reply	other threads:[~2021-04-15  1:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15  1:50 Douglas Gilbert [this message]
2021-04-15  9:15 ` [PATCH] scsi_debug: fix cmd_per_lun, set to max_queue John Garry
2021-04-16  1:46   ` Ming Lei
2021-04-16  8:17     ` John Garry
2021-04-16  8:26       ` Ming Lei
2021-04-16  8:33         ` John Garry
2021-04-16  8:50           ` Ming Lei
2021-04-16  9:07             ` John Garry
2021-04-16  9:12 ` John Garry
2021-04-16 16:32   ` Douglas Gilbert
2021-04-29 13:17     ` John Garry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210415015031.607153-1-dgilbert@interlog.com \
    --to=dgilbert@interlog.com \
    --cc=axboe@kernel.dk \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=john.garry@huawei.com \
    --cc=kashyap.desai@broadcom.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox