Re: [PATCH RFC v3 06/41] virtio_scsi: use reserved commands for TMF

[Hannes Reinecke <hare@xxxxxxx>]

On 5/4/20 11:25 AM, Ming Lei wrote:
> On Thu, Apr 30, 2020 at 03:18:29PM +0200, Hannes Reinecke wrote:
> > From: Hannes Reinecke <hare@xxxxxxxx>
> >
> > Set two commands aside for TMF, and use reserved commands to issue
> > TMFs. With that we can drop the TMF memory pool.
> >
> > Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> > ---
> >   drivers/scsi/virtio_scsi.c | 105 ++++++++++++++++++---------------------------
> >   1 file changed, 41 insertions(+), 64 deletions(-)
> >
> > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> > index 0e0910c5b942..26054c29d897 100644
> > --- a/drivers/scsi/virtio_scsi.c
> > +++ b/drivers/scsi/virtio_scsi.c
> > @@ -35,10 +35,10 @@
> >   #define VIRTIO_SCSI_MEMPOOL_SZ 64
> >   #define VIRTIO_SCSI_EVENT_LEN 8
> >   #define VIRTIO_SCSI_VQ_BASE 2
> > +#define VIRTIO_SCSI_RESERVED_CMDS 2
> >   
> >   /* Command queue element */
> >   struct virtio_scsi_cmd {
> > -	struct scsi_cmnd *sc;
> >   	struct completion *comp;
> >   	union {
> >   		struct virtio_scsi_cmd_req       cmd;
> > @@ -86,9 +86,6 @@ struct virtio_scsi {
> >   	struct virtio_scsi_vq req_vqs[];
> >   };
> >   
> > -static struct kmem_cache *virtscsi_cmd_cache;
> > -static mempool_t *virtscsi_cmd_pool;
> > -
> >   static inline struct Scsi_Host *virtio_scsi_host(struct virtio_device *vdev)
> >   {
> >   	return vdev->priv;
> > @@ -108,7 +105,7 @@ static void virtscsi_compute_resid(struct scsi_cmnd *sc, u32 resid)
> >   static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf)
> >   {
> >   	struct virtio_scsi_cmd *cmd = buf;
> > -	struct scsi_cmnd *sc = cmd->sc;
> > +	struct scsi_cmnd *sc = scsi_cmd_from_priv(cmd);
> >   	struct virtio_scsi_cmd_resp *resp = &cmd->resp.cmd;
> >   
> >   	dev_dbg(&sc->device->sdev_gendev,
> > @@ -406,7 +403,7 @@ static int __virtscsi_add_cmd(struct virtqueue *vq,
> >   			    struct virtio_scsi_cmd *cmd,
> >   			    size_t req_size, size_t resp_size)
> >   {
> > -	struct scsi_cmnd *sc = cmd->sc;
> > +	struct scsi_cmnd *sc = scsi_cmd_from_priv(cmd);
> >   	struct scatterlist *sgs[6], req, resp;
> >   	struct sg_table *out, *in;
> >   	unsigned out_num = 0, in_num = 0;
> > @@ -557,8 +554,6 @@ static int virtscsi_queuecommand(struct Scsi_Host *shost,
> >   	dev_dbg(&sc->device->sdev_gendev,
> >   		"cmd %p CDB: %#02x\n", sc, sc->cmnd[0]);
> >   
> > -	cmd->sc = sc;
> > -
> >   	BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE);
> >   
> >   #ifdef CONFIG_BLK_DEV_INTEGRITY
> > @@ -590,17 +585,17 @@ static int virtscsi_queuecommand(struct Scsi_Host *shost,
> >   static int virtscsi_tmf(struct virtio_scsi *vscsi, struct virtio_scsi_cmd *cmd)
> >   {
> >   	DECLARE_COMPLETION_ONSTACK(comp);
> > -	int ret = FAILED;
> >   
> >   	cmd->comp = &comp;
> > +
> >   	if (virtscsi_add_cmd(&vscsi->ctrl_vq, cmd,
> >   			      sizeof cmd->req.tmf, sizeof cmd->resp.tmf, true) < 0)
>
> virtscsi uses dedicated ->ctrl_vq to send TMF, and ->ctrl_vq isn't
> nothing to do with vqs(->vqs[]) for sending IO request.
Indeed, you are right.
We should be handling this by adding a virtual LUN corresponding the the 
ctrl_vq and allocate commands from there.

Will be updating for the next round.

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