On 12/3/24 11:15 AM, Mike Christie wrote:
> If we can't allocate mem to map in data for a request or can't find
> a tag for a command, we currently drop the command. This leads to the
> error handler running to clean it up. Instead of dropping the command
> this has us return an error telling the initiator that it queued more
> commands than we can handle. The initiator will then reduce how many
> commands it will send us and retry later.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
> drivers/vhost/scsi.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 99e008219e6c..ad7c778632f9 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -629,7 +629,7 @@ vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
>
> tag = sbitmap_get(&svq->scsi_tags);
> if (tag < 0) {
> - pr_err("Unable to obtain tag for vhost_scsi_cmd\n");
> + pr_warn_once("Guest sent too many cmds. Returning TASK_SET_FULL.\n");
> return ERR_PTR(-ENOMEM);
> }
>
> @@ -928,6 +928,24 @@ static void vhost_scsi_target_queue_cmd(struct vhost_scsi_cmd *cmd)
> target_submit(se_cmd);
> }
>
> +static void
> +vhost_scsi_send_status(struct vhost_scsi *vs, struct vhost_virtqueue *vq,
> + int head, unsigned int out, u8 status)
> +{
> + struct virtio_scsi_cmd_resp __user *resp;
> + struct virtio_scsi_cmd_resp rsp;
> + int ret;
> +
> + memset(&rsp, 0, sizeof(rsp));
> + rsp.status = status;
> + resp = vq->iov[out].iov_base;
> + ret = __copy_to_user(resp, &rsp, sizeof(rsp));
> + if (!ret)
> + vhost_add_used_and_signal(&vs->dev, vq, head, 0);
> + else
> + pr_err("Faulted on virtio_scsi_cmd_resp\n");
> +}
> +
When I re-base my patchset on top of the most recent change ...
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=linux-next
... I notice this is the same issue as what the patch is going to fix:
[PATCH 9/9] vhost-scsi: Fix vhost_scsi_send_bad_target()
https://lore.kernel.org/all/20250207184212.20831-10-dongli.zhang@xxxxxxxxxx/
We cannot assume the response takes only one iovec.
We may need something like (not tested):
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a4cbdc607fa..dd9614464732 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -994,18 +994,25 @@ static void vhost_scsi_target_queue_cmd(struct
vhost_scsi_nexus *nexus,
static void
vhost_scsi_send_status(struct vhost_scsi *vs, struct vhost_virtqueue *vq,
- int head, unsigned int out, u8 status)
+ struct vhost_scsi_ctx *vc, u8 status)
{
- struct virtio_scsi_cmd_resp __user *resp;
- struct virtio_scsi_cmd_resp rsp;
+ struct virtio_scsi_cmd_resp resp;
+ struct iov_iter iov_iter;
+ size_t resp_size;
int ret;
- memset(&rsp, 0, sizeof(rsp));
- rsp.status = status;
- resp = vq->iov[out].iov_base;
- ret = __copy_to_user(resp, &rsp, sizeof(rsp));
- if (!ret)
- vhost_add_used_and_signal(&vs->dev, vq, head, 0);
+ resp_size = sizeof(struct virtio_scsi_cmd_resp);
+
+ memset(&resp, 0, sizeof(resp));
+ resp.status = status;
+
+ iov_iter_init(&iov_iter, ITER_DEST, &vq->iov[vc->out], vc->in,
+ resp_size);
+
+ ret = copy_to_iter(&resp, resp_size, &iov_iter);
+
+ if (likely(ret == resp_size))
+ vhost_add_used_and_signal(&vs->dev, vq, vc->head, 0);
else
pr_err("Faulted on virtio_scsi_cmd_resp\n");
}
@@ -1392,7 +1399,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
else if (ret == -EIO)
vhost_scsi_send_bad_target(vs, vq, vc.head, vc.out);
else if (ret == -ENOMEM)
- vhost_scsi_send_status(vs, vq, vc.head, vc.out,
+ vhost_scsi_send_status(vs, vq, &vc,
SAM_STAT_TASK_SET_FULL);
} while (likely(!vhost_exceeds_weight(vq, ++c, 0)));
out:
I will add it when to re-base my patchset.
Thank you very much!
Dongli Zhang
