On Thu, Apr 15, 2021 at 10:15:27AM +0100, John Garry wrote: > This looks ok. > > Apart from this, I tested linux-next (without this patch) - which includes > Ming's changes to replace sdev-->device_busy with sbitmap - and, as > expected, it has the issue. > > So I think it is also worth having this to stop this happening elsewhere: > > ------>8------- > > Subject: [PATCH] scsi: core: Cap initial sdev queue depth at Shost.can_queue > > Function sdev_store_queue_depth() enforces that the sdev queue depth cannot > exceed Shost.can_queue. > > However, the LLDD may still set cmd_per_lun > can_queue, which would lead to > an initial sdev queue depth greater than can_queue. > > Stop this happened by capping initial sdev queue depth at can_queue. > > <insert credits> > Signed-off-by: John Garry <john.garry@xxxxxxxxxx> > > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c > index 9af50e6f94c4..fec6c17ff37c 100644 > --- a/drivers/scsi/scsi_scan.c > +++ b/drivers/scsi/scsi_scan.c > @@ -218,6 +218,7 @@ static struct scsi_device *scsi_alloc_sdev(struct > scsi_target *starget, > struct scsi_device *sdev; > int display_failure_msg = 1, ret; > struct Scsi_Host *shost = dev_to_shost(starget->dev.parent); > + int depth; > > sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size, > GFP_KERNEL); > @@ -276,8 +277,13 @@ static struct scsi_device *scsi_alloc_sdev(struct > scsi_target *starget, > WARN_ON_ONCE(!blk_get_queue(sdev->request_queue)); > sdev->request_queue->queuedata = sdev; > > - scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun ? > - sdev->host->cmd_per_lun : 1); > + if (sdev->host->cmd_per_lun) > + depth = min_t(int, sdev->host->cmd_per_lun, > + sdev->host->can_queue); > + else > + depth = 1; > + > + scsi_change_queue_depth(sdev, depth); 'cmd_per_lun' should have been set as correct from the beginning instead of capping it for changing queue depth: diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 697c09ef259b..0d9954eabbb8 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -414,7 +414,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) shost->can_queue = sht->can_queue; shost->sg_tablesize = sht->sg_tablesize; shost->sg_prot_tablesize = sht->sg_prot_tablesize; - shost->cmd_per_lun = sht->cmd_per_lun; + shost->cmd_per_lun = min_t(int, sht->cmd_per_lun, shost->can_queue); shost->no_write_same = sht->no_write_same; shost->host_tagset = sht->host_tagset; Thanks, Ming
