Skip to content

Commit 57f7225

Browse files
johnpgarrymartinkpetersen
authored andcommitted
scsi: scsi_debug: Only allow sdebug_max_queue be modified when no shosts
The shost->can_queue value is initially used to set per-HW queue context tag depth in the block layer. This ensures that the shost is not sent too many commands which it can deal with. However lowering sdebug_max_queue separately means that we can easily overload the shost, as in the following example: $ cat /sys/bus/pseudo/drivers/scsi_debug/max_queue 192 $ cat /sys/class/scsi_host/host0/can_queue 192 $ echo 100 > /sys/bus/pseudo/drivers/scsi_debug/max_queue $ cat /sys/class/scsi_host/host0/can_queue 192 $ fio --filename=/dev/sda --direct=1 --rw=read --bs=4k --iodepth=256 --runtime=1200 --numjobs=10 --time_based --group_reporting --name=iops-test-job --eta-newline=1 --readonly --ioengine=io_uring --hipri --exitall_on_error iops-test-job: (g=0): rw=read, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=io_uring, iodepth=256 ... fio-3.28 Starting 10 processes [ 111.269885] scsi_io_completion_action: 400 callbacks suppressed [ 111.269885] blk_print_req_error: 400 callbacks suppressed [ 111.269889] I/O error, dev sda, sector 440 op 0x0:(READ) flags 0x1200000 phys_seg 1 prio class 2 [ 111.269892] sd 0:0:0:0: [sda] tag#132 FAILED Result: hostbyte=DID_ABORT driverbyte=DRIVER_OK cmd_age=0s [ 111.269897] sd 0:0:0:0: [sda] tag#132 CDB: Read(10) 28 00 00 00 01 68 00 00 08 00 [ 111.277058] I/O error, dev sda, sector 360 op 0x0:(READ) flags 0x1200000 phys_seg 1 prio class 2 [...] Ensure that this cannot happen by allowing sdebug_max_queue be modified only when we have no shosts. As such, any shost->can_queue value will match sdebug_max_queue, and sdebug_max_queue cannot be modified separately. Since retired_max_queue is no longer set, remove support. Continue to apply the restriction that sdebug_host_max_queue cannot be modified when sdebug_host_max_queue is set. Adding support for that would mean extra code, and no one has complained about this restriction previously. A command like the following may be used to remove a shost: echo -1 > /sys/bus/pseudo/drivers/scsi_debug/add_host Signed-off-by: John Garry <john.g.garry@oracle.com> Link: https://lore.kernel.org/r/20230327074310.1862889-11-john.g.garry@oracle.com Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 12f3eef commit 57f7225

1 file changed

Lines changed: 6 additions & 61 deletions

File tree

drivers/scsi/scsi_debug.c

Lines changed: 6 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,6 @@ static int sdebug_max_luns = DEF_MAX_LUNS;
762762
static int sdebug_max_queue = SDEBUG_CANQUEUE; /* per submit queue */
763763
static unsigned int sdebug_medium_error_start = OPT_MEDIUM_ERR_ADDR;
764764
static int sdebug_medium_error_count = OPT_MEDIUM_ERR_NUM;
765-
static atomic_t retired_max_queue; /* if > 0 then was prior max_queue */
766765
static int sdebug_ndelay = DEF_NDELAY; /* if > 0 then unit is nanoseconds */
767766
static int sdebug_no_lun_0 = DEF_NO_LUN_0;
768767
static int sdebug_no_uld;
@@ -4928,7 +4927,6 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
49284927
{
49294928
struct sdebug_queued_cmd *sqcp = container_of(sd_dp, struct sdebug_queued_cmd, sd_dp);
49304929
int qc_idx;
4931-
int retiring = 0;
49324930
unsigned long flags, iflags;
49334931
struct scsi_cmnd *scp = sqcp->scmd;
49344932
struct sdebug_scsi_cmd *sdsc;
@@ -4959,9 +4957,6 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
49594957
sd_dp->aborted = false;
49604958
ASSIGN_QUEUED_CMD(scp, NULL);
49614959

4962-
if (unlikely(atomic_read(&retired_max_queue) > 0))
4963-
retiring = 1;
4964-
49654960
sqp->qc_arr[qc_idx] = NULL;
49664961
if (unlikely(!test_and_clear_bit(qc_idx, sqp->in_use_bm))) {
49674962
spin_unlock_irqrestore(&sdsc->lock, flags);
@@ -4970,23 +4965,6 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp)
49704965
goto out;
49714966
}
49724967

4973-
if (unlikely(retiring)) { /* user has reduced max_queue */
4974-
int k, retval;
4975-
4976-
retval = atomic_read(&retired_max_queue);
4977-
if (qc_idx >= retval) {
4978-
spin_unlock_irqrestore(&sdsc->lock, flags);
4979-
spin_unlock_irqrestore(&sqp->qc_lock, iflags);
4980-
pr_err("index %d too large\n", retval);
4981-
goto out;
4982-
}
4983-
k = find_last_bit(sqp->in_use_bm, retval);
4984-
if ((k < sdebug_max_queue) || (k == retval))
4985-
atomic_set(&retired_max_queue, 0);
4986-
else
4987-
atomic_set(&retired_max_queue, k + 1);
4988-
}
4989-
49904968
spin_unlock_irqrestore(&sdsc->lock, flags);
49914969
spin_unlock_irqrestore(&sqp->qc_lock, iflags);
49924970

@@ -6431,29 +6409,18 @@ static ssize_t max_queue_show(struct device_driver *ddp, char *buf)
64316409
static ssize_t max_queue_store(struct device_driver *ddp, const char *buf,
64326410
size_t count)
64336411
{
6434-
int j, n, k, a;
6435-
struct sdebug_queue *sqp;
6412+
int n;
64366413

64376414
if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n > 0) &&
64386415
(n <= SDEBUG_CANQUEUE) &&
64396416
(sdebug_host_max_queue == 0)) {
64406417
mutex_lock(&sdebug_host_list_mutex);
6441-
block_unblock_all_queues(true);
6442-
k = 0;
6443-
for (j = 0, sqp = sdebug_q_arr; j < submit_queues;
6444-
++j, ++sqp) {
6445-
a = find_last_bit(sqp->in_use_bm, SDEBUG_CANQUEUE);
6446-
if (a > k)
6447-
k = a;
6448-
}
6449-
sdebug_max_queue = n;
6450-
if (k == SDEBUG_CANQUEUE)
6451-
atomic_set(&retired_max_queue, 0);
6452-
else if (k >= n)
6453-
atomic_set(&retired_max_queue, k + 1);
6418+
6419+
/* We may only change sdebug_max_queue when we have no shosts */
6420+
if (list_empty(&sdebug_host_list))
6421+
sdebug_max_queue = n;
64546422
else
6455-
atomic_set(&retired_max_queue, 0);
6456-
block_unblock_all_queues(false);
6423+
count = -EBUSY;
64576424
mutex_unlock(&sdebug_host_list_mutex);
64586425
return count;
64596426
}
@@ -6882,7 +6849,6 @@ static int __init scsi_debug_init(void)
68826849

68836850
ramdisk_lck_a[0] = &atomic_rw;
68846851
ramdisk_lck_a[1] = &atomic_rw2;
6885-
atomic_set(&retired_max_queue, 0);
68866852

68876853
if (sdebug_ndelay >= 1000 * 1000 * 1000) {
68886854
pr_warn("ndelay must be less than 1 second, ignored\n");
@@ -7520,7 +7486,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
75207486
struct sdebug_queue *sqp;
75217487
unsigned long flags;
75227488
int queue_num = data->queue_num;
7523-
bool retiring = false;
75247489
int qc_idx;
75257490
ktime_t time;
75267491

@@ -7554,9 +7519,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
75547519
return true;
75557520
}
75567521

7557-
if (unlikely(atomic_read(&retired_max_queue) > 0))
7558-
retiring = true;
7559-
75607522
qc_idx = sd_dp->sqa_idx;
75617523
sqp->qc_arr[qc_idx] = NULL;
75627524
if (unlikely(!test_and_clear_bit(qc_idx, sqp->in_use_bm))) {
@@ -7567,23 +7529,6 @@ static bool sdebug_blk_mq_poll_iter(struct request *rq, void *opaque)
75677529
return true;
75687530
}
75697531

7570-
if (unlikely(retiring)) { /* user has reduced max_queue */
7571-
int k, retval = atomic_read(&retired_max_queue);
7572-
7573-
if (qc_idx >= retval) {
7574-
pr_err("index %d too large\n", retval);
7575-
spin_unlock_irqrestore(&sdsc->lock, flags);
7576-
sdebug_free_queued_cmd(sqcp);
7577-
return true;
7578-
}
7579-
7580-
k = find_last_bit(sqp->in_use_bm, retval);
7581-
if ((k < sdebug_max_queue) || (k == retval))
7582-
atomic_set(&retired_max_queue, 0);
7583-
else
7584-
atomic_set(&retired_max_queue, k + 1);
7585-
}
7586-
75877532
ASSIGN_QUEUED_CMD(cmd, NULL);
75887533
spin_unlock_irqrestore(&sdsc->lock, flags);
75897534

0 commit comments

Comments
 (0)