On Fri, Apr 16, 2021 at 09:17:09AM +0100, John Garry wrote: > On 16/04/2021 02:46, Ming Lei wrote: > > > 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; > > My concern here is that it is a common pattern in LLDDs to overwrite the > initial shost member values between scsi_host_alloc() and scsi_add_host(). OK, then can we move the fix into beginning of scsi_add_host()? Thanks, Ming
