On Fri, Apr 16, 2021 at 09:33:48AM +0100, John Garry wrote: > On 16/04/2021 09:26, Ming Lei wrote: > > > > '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()? > > I suppose that would be ok, but we don't do much sanitizing shost values at > that point. Apart from failing can_queue == 0. .can_queue has been finalized in scsi_add_host(), since it will be used for setting tagset, so .can_queue is reliable at that time. >I suppose failing can_queue < cmd_per_lun could also be added. That will fail add host for scsi_debug simply. Thanks, Ming
