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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> -		goto out;
> +		return FAILED;
>  
>  	wait_for_completion(&comp);
>  	if (cmd->resp.tmf.response == VIRTIO_SCSI_S_OK ||
>  	    cmd->resp.tmf.response == VIRTIO_SCSI_S_FUNCTION_SUCCEEDED)
> -		ret = SUCCESS;
> +		return SUCCESS;
>  
>  	/*
>  	 * The spec guarantees that all requests related to the TMF have
> @@ -613,33 +608,36 @@ static int virtscsi_tmf(struct virtio_scsi *vscsi, struct virtio_scsi_cmd *cmd)
>  	 * REQ_ATOM_COMPLETE has been set.
>  	 */
>  	virtscsi_poll_requests(vscsi);
> -
> -out:
> -	mempool_free(cmd, virtscsi_cmd_pool);
> -	return ret;
> +	return FAILED;
>  }
>  
>  static int virtscsi_device_reset(struct scsi_cmnd *sc)
>  {
> +	struct scsi_device *sdev = sc->device;
> +	struct scsi_cmnd *reset_sc;
>  	struct virtio_scsi *vscsi = shost_priv(sc->device->host);
>  	struct virtio_scsi_cmd *cmd;
> +	int rc;
>  
> -	sdev_printk(KERN_INFO, sc->device, "device reset\n");
> -	cmd = mempool_alloc(virtscsi_cmd_pool, GFP_NOIO);
> -	if (!cmd)
> +	sdev_printk(KERN_INFO, sdev, "device reset\n");
> +	reset_sc = scsi_get_reserved_cmd(sdev, DMA_NONE);

As commented above, virtio scsi doesn't use IO vqs to send TMF, so the above
reserved command isn't necessary.

There might be other example which do not require IO tag to
send reset command.

Thanks, 
Ming




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux