Re: [PATCH RFC v3 29/41] snic: use reserved commands

[Hannes Reinecke <hare@xxxxxxx>]

On 5/2/20 5:19 AM, Ming Lei wrote:
> On Thu, Apr 30, 2020 at 03:18:52PM +0200, Hannes Reinecke wrote:
> > From: Hannes Reinecke <hare@xxxxxxxx>
> >
> > Use a reserved command for host and device reset.
> >
> > Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> > ---
> >   drivers/scsi/snic/snic.h      |   4 +-
> >   drivers/scsi/snic/snic_main.c |   8 +++
> >   drivers/scsi/snic/snic_scsi.c | 140 +++++++++++++++++-------------------------
> >   3 files changed, 66 insertions(+), 86 deletions(-)
> >
> > diff --git a/drivers/scsi/snic/snic.h b/drivers/scsi/snic/snic.h
> > index de0ab5fc8474..7dc529ae8a90 100644
> > --- a/drivers/scsi/snic/snic.h
> > +++ b/drivers/scsi/snic/snic.h
> > @@ -59,7 +59,6 @@
> >    */
> >   #define SNIC_TAG_ABORT		BIT(30)		/* Tag indicating abort */
> >   #define SNIC_TAG_DEV_RST	BIT(29)		/* Tag for device reset */
> > -#define SNIC_TAG_IOCTL_DEV_RST	BIT(28)		/* Tag for User Device Reset */
> >   #define SNIC_TAG_MASK		(BIT(24) - 1)	/* Mask for lookup */
> >   #define SNIC_NO_TAG		-1
> >   
> > @@ -278,6 +277,7 @@ struct snic {
> >   
> >   	/* Scsi Host info */
> >   	struct Scsi_Host *shost;
> > +	struct scsi_device *shost_dev;
> >   
> >   	/* vnic related structures */
> >   	struct vnic_dev_bar bar0;
> > @@ -380,7 +380,7 @@ int snic_queuecommand(struct Scsi_Host *, struct scsi_cmnd *);
> >   int snic_abort_cmd(struct scsi_cmnd *);
> >   int snic_device_reset(struct scsi_cmnd *);
> >   int snic_host_reset(struct scsi_cmnd *);
> > -int snic_reset(struct Scsi_Host *, struct scsi_cmnd *);
> > +int snic_reset(struct Scsi_Host *);
> >   void snic_shutdown_scsi_cleanup(struct snic *);
> >   
> >   
> > diff --git a/drivers/scsi/snic/snic_main.c b/drivers/scsi/snic/snic_main.c
> > index 14f4ce665e58..f520da64ec8e 100644
> > --- a/drivers/scsi/snic/snic_main.c
> > +++ b/drivers/scsi/snic/snic_main.c
> > @@ -303,6 +303,7 @@ static int
> >   snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
> >   {
> >   	int ret = 0;
> > +	struct snic *snic = shost_priv(shost);
> >   
> >   	ret = scsi_add_host(shost, &pdev->dev);
> >   	if (ret) {
> > @@ -313,6 +314,12 @@ snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
> >   		return ret;
> >   	}
> >   
> > +	snic->shost_dev = scsi_get_virtual_dev(shost, 1, 0);
> > +	if (!snic->shost_dev) {
> > +		SNIC_HOST_ERR(shost,
> > +			      "snic: scsi_get_virtual_dev failed\n");
> > +		return -ENOMEM;
> > +	}
> >   	SNIC_BUG_ON(shost->work_q != NULL);
> >   	snprintf(shost->work_q_name, sizeof(shost->work_q_name), "scsi_wq_%d",
> >   		 shost->host_no);
> > @@ -385,6 +392,7 @@ snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >   
> >   		goto prob_end;
> >   	}
> > +	shost->nr_reserved_cmds = 2;
>
> Not see .can_queue is increased by 2 in this patch, please comment on
> the reason. Otherwise, IO performance drop may be caused.
snic is requiring a tag for both LU reset and Host reset.
We do require one tag for SCSI EH, and I'm setting aside another one for 
ioctl commands.
Will be adding a comment here explaining things.

Cheers,

Hannes
--
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@xxxxxxx                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer