Re: [PATCH] scsi_debug: fix cmd_per_lun, set to max_queue

[Ming Lei <ming.lei@redhat.com>]

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