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: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