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 = ∁
> +
> if (virtscsi_add_cmd(&vscsi->ctrl_vq, cmd,
> sizeof cmd->req.tmf, sizeof cmd->resp.tmf, true) < 0)
> - 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);
> + if (!reset_sc)
> return FAILED;
> -
> + cmd = scsi_cmd_priv(reset_sc);
> memset(cmd, 0, sizeof(*cmd));
> cmd->req.tmf = (struct virtio_scsi_ctrl_tmf_req){
> .type = VIRTIO_SCSI_T_TMF,
> .subtype = cpu_to_virtio32(vscsi->vdev,
> VIRTIO_SCSI_T_TMF_LOGICAL_UNIT_RESET),
> .lun[0] = 1,
> - .lun[1] = sc->device->id,
> - .lun[2] = (sc->device->lun >> 8) | 0x40,
> - .lun[3] = sc->device->lun & 0xff,
> + .lun[1] = sdev->id,
> + .lun[2] = (sdev->lun >> 8) | 0x40,
> + .lun[3] = sdev->lun & 0xff,
> };
> - return virtscsi_tmf(vscsi, cmd);
> + rc = virtscsi_tmf(vscsi, cmd);
> + scsi_put_reserved_cmd(reset_sc);
> +
> + return rc;
> }
>
> static int virtscsi_device_alloc(struct scsi_device *sdevice)
> @@ -679,25 +677,31 @@ static int virtscsi_change_queue_depth(struct scsi_device *sdev, int qdepth)
>
> static int virtscsi_abort(struct scsi_cmnd *sc)
> {
> - struct virtio_scsi *vscsi = shost_priv(sc->device->host);
> + struct scsi_device *sdev = sc->device;
> + struct scsi_cmnd *reset_sc;
> + struct virtio_scsi *vscsi = shost_priv(sdev->host);
> struct virtio_scsi_cmd *cmd;
> + int rc;
>
> scmd_printk(KERN_INFO, sc, "abort\n");
> - cmd = mempool_alloc(virtscsi_cmd_pool, GFP_NOIO);
> - if (!cmd)
> + reset_sc = scsi_get_reserved_cmd(sdev, DMA_NONE);
> + if (!reset_sc)
> return FAILED;
> + cmd = scsi_cmd_priv(reset_sc);
>
> memset(cmd, 0, sizeof(*cmd));
> cmd->req.tmf = (struct virtio_scsi_ctrl_tmf_req){
> .type = VIRTIO_SCSI_T_TMF,
> .subtype = VIRTIO_SCSI_T_TMF_ABORT_TASK,
> .lun[0] = 1,
> - .lun[1] = sc->device->id,
> - .lun[2] = (sc->device->lun >> 8) | 0x40,
> - .lun[3] = sc->device->lun & 0xff,
> + .lun[1] = sdev->id,
> + .lun[2] = (sdev->lun >> 8) | 0x40,
> + .lun[3] = sdev->lun & 0xff,
> .tag = cpu_to_virtio64(vscsi->vdev, (unsigned long)sc),
> };
> - return virtscsi_tmf(vscsi, cmd);
> + rc = virtscsi_tmf(vscsi, cmd);
> + scsi_put_reserved_cmd(reset_sc);
> + return rc;
> }
>
> static int virtscsi_map_queues(struct Scsi_Host *shost)
> @@ -865,6 +869,11 @@ static int virtscsi_probe(struct virtio_device *vdev)
> goto virtscsi_init_failed;
>
> shost->can_queue = virtqueue_get_vring_size(vscsi->req_vqs[0].vq);
> + shost->can_queue -= VIRTIO_SCSI_RESERVED_CMDS;
I guess it should have been '+=', right?
Thanks,
Ming
