Re: [PATCH RFC 2/2] vhost: support urgent descriptors

From: Jason Wang
Date: Fri Sep 19 2014 - 06:36:21 EST


On 07/01/2014 06:49 PM, Michael S. Tsirkin wrote:
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
> drivers/vhost/vhost.h | 19 +++++++++++++------
> drivers/vhost/net.c | 30 +++++++++++++++++++++---------
> drivers/vhost/scsi.c | 23 +++++++++++++++--------
> drivers/vhost/test.c | 5 +++--
> drivers/vhost/vhost.c | 23 ++++++++++++++++-------
> 5 files changed, 68 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 3eda654..61ca542 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -96,6 +96,9 @@ struct vhost_virtqueue {
> /* Last used index value we have signalled on */
> bool signalled_used_valid;
>
> + /* Urgent descriptor was used */
> + bool urgent;
> +
> /* Log writes to used structure. */
> bool log_used;
> u64 log_addr;
> @@ -138,20 +141,24 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp);
> int vhost_vq_access_ok(struct vhost_virtqueue *vq);
> int vhost_log_access_ok(struct vhost_dev *);
>
> -int vhost_get_vq_desc(struct vhost_virtqueue *,
> +int vhost_get_vq_desc(struct vhost_virtqueue *, bool *urgent,
> struct iovec iov[], unsigned int iov_count,
> unsigned int *out_num, unsigned int *in_num,
> struct vhost_log *log, unsigned int *log_num);
> void vhost_discard_vq_desc(struct vhost_virtqueue *, int n);
>
> int vhost_init_used(struct vhost_virtqueue *);
> -int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len);
> -int vhost_add_used_n(struct vhost_virtqueue *, struct vring_used_elem *heads,
> - unsigned count);
> -void vhost_add_used_and_signal(struct vhost_dev *, struct vhost_virtqueue *,
> +int vhost_add_used(struct vhost_virtqueue *, bool urgent,
> + unsigned int head, int len);
> +int vhost_add_used_n(struct vhost_virtqueue *, bool urgent,
> + struct vring_used_elem *heads, unsigned count);
> +void vhost_add_used_and_signal(struct vhost_dev *,
> + struct vhost_virtqueue *,
> + bool urgent,
> unsigned int id, int len);
> void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
> - struct vring_used_elem *heads, unsigned count);
> + bool urgent,
> + struct vring_used_elem *heads, unsigned count);
> void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
> void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 8dae2f7..5f0567f 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -48,9 +48,11 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> * status internally; used for zerocopy tx only.
> */
> /* Lower device DMA failed */
> -#define VHOST_DMA_FAILED_LEN 3
> +#define VHOST_DMA_FAILED_LEN 4
> /* Lower device DMA done */
> -#define VHOST_DMA_DONE_LEN 2
> +#define VHOST_DMA_DONE_LEN 3
> +/* Lower device DMA in progress, urgent bit set */
> +#define VHOST_DMA_URGENT 2
> /* Lower device DMA in progress */
> #define VHOST_DMA_IN_PROGRESS 1
> /* Buffer unused */
> @@ -284,11 +286,13 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
> container_of(vq, struct vhost_net_virtqueue, vq);
> int i, add;
> int j = 0;
> + bool urgent = false;
>
> for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
> if (vq->heads[i].len == VHOST_DMA_FAILED_LEN)
> vhost_net_tx_err(net);
> if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
> + urgent = urgent || vq->heads[i].len == VHOST_DMA_URGENT;

A problem is len was either VHOST_DMA_DONE_LEN or VHOST_DMA_FAILED_LEN
here. Looks like we need another new VHOST_DMA_DONE_LEN_URGENT and
change the len to this value if it was an urgent descriptor in zerocopy
callback.
> vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
> ++j;
> } else
> @@ -296,7 +300,7 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
> }
> while (j) {
> add = min(UIO_MAXIOV - nvq->done_idx, j);
> - vhost_add_used_and_signal_n(vq->dev, vq,
> + vhost_add_used_and_signal_n(vq->dev, vq, urgent,
> &vq->heads[nvq->done_idx], add);
> nvq->done_idx = (nvq->done_idx + add) % UIO_MAXIOV;
> j -= add;
> @@ -363,6 +367,7 @@ static void handle_tx(struct vhost_net *net)
> zcopy = nvq->ubufs;
>
> for (;;) {
> + bool urgent;
> /* Release DMAs done buffers first */
> if (zcopy)
> vhost_zerocopy_signal_used(net, vq);
> @@ -374,7 +379,7 @@ static void handle_tx(struct vhost_net *net)
> % UIO_MAXIOV == nvq->done_idx))
> break;
>
> - head = vhost_get_vq_desc(vq, vq->iov,
> + head = vhost_get_vq_desc(vq, &urgent, vq->iov,
> ARRAY_SIZE(vq->iov),
> &out, &in,
> NULL, NULL);
> @@ -417,7 +422,8 @@ static void handle_tx(struct vhost_net *net)
> ubuf = nvq->ubuf_info + nvq->upend_idx;
>
> vq->heads[nvq->upend_idx].id = head;
> - vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
> + vq->heads[nvq->upend_idx].len = urgent ?
> + VHOST_DMA_IN_PROGRESS : VHOST_DMA_URGENT;

I think you mean VHOST_DMA_URGENT : VHOST_DMA_IN_PROGRESS here.

With all the changes above, zercopy mode works as expected. I will post
the driver part code and result next week for early review.

Thanks
> ubuf->callback = vhost_zerocopy_callback;
> ubuf->ctx = nvq->ubufs;
> ubuf->desc = nvq->upend_idx;
> @@ -445,7 +451,7 @@ static void handle_tx(struct vhost_net *net)
> pr_debug("Truncated TX packet: "
> " len %d != %zd\n", err, len);
> if (!zcopy_used)
> - vhost_add_used_and_signal(&net->dev, vq, head, 0);
> + vhost_add_used_and_signal(&net->dev, vq, urgent, head, 0);
> else
> vhost_zerocopy_signal_used(net, vq);
> total_len += len;
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/