ACK: [SRU][Zesty][PATCH 1/1][v2]UBUNTU: SAUCE: Revert "vhost: cache used event for better performance"

[Colin Ian King]

On 01/09/17 17:23, Joseph Salisbury wrote:
> BugLink: http://bugs.launchpad.net/bugs/1711251
> 
> This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92.
> ---
>  drivers/vhost/vhost.c | 28 ++++++----------------------
>  drivers/vhost/vhost.h |  3 ---
>  2 files changed, 6 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 8f99fe0..8d05a40 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -290,7 +290,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>  	vq->avail = NULL;
>  	vq->used = NULL;
>  	vq->last_avail_idx = 0;
> -	vq->last_used_event = 0;
>  	vq->avail_idx = 0;
>  	vq->last_used_idx = 0;
>  	vq->signalled_used = 0;
> @@ -1325,7 +1324,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
>  			r = -EINVAL;
>  			break;
>  		}
> -		vq->last_avail_idx = vq->last_used_event = s.num;
> +		vq->last_avail_idx = s.num;
>  		/* Forget the cached index value. */
>  		vq->avail_idx = vq->last_avail_idx;
>  		break;
> @@ -2157,6 +2156,10 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  	__u16 old, new;
>  	__virtio16 event;
>  	bool v;
> +	/* Flush out used index updates. This is paired
> +	 * with the barrier that the Guest executes when enabling
> +	 * interrupts. */
> +	smp_mb();
>  
>  	if (vhost_has_feature(vq, VIRTIO_F_NOTIFY_ON_EMPTY) &&
>  	    unlikely(vq->avail_idx == vq->last_avail_idx))
> @@ -2164,10 +2167,6 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  
>  	if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
>  		__virtio16 flags;
> -		/* Flush out used index updates. This is paired
> -		 * with the barrier that the Guest executes when enabling
> -		 * interrupts. */
> -		smp_mb();
>  		if (vhost_get_user(vq, flags, &vq->avail->flags)) {
>  			vq_err(vq, "Failed to get flags");
>  			return true;
> @@ -2182,26 +2181,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  	if (unlikely(!v))
>  		return true;
>  
> -	/* We're sure if the following conditions are met, there's no
> -	 * need to notify guest:
> -	 * 1) cached used event is ahead of new
> -	 * 2) old to new updating does not cross cached used event. */
> -	if (vring_need_event(vq->last_used_event, new + vq->num, new) &&
> -	    !vring_need_event(vq->last_used_event, new, old))
> -		return false;
> -
> -	/* Flush out used index updates. This is paired
> -	 * with the barrier that the Guest executes when enabling
> -	 * interrupts. */
> -	smp_mb();
> -
>  	if (vhost_get_user(vq, event, vhost_used_event(vq))) {
>  		vq_err(vq, "Failed to get used event idx");
>  		return true;
>  	}
> -	vq->last_used_event = vhost16_to_cpu(vq, event);
> -
> -	return vring_need_event(vq->last_used_event, new, old);
> +	return vring_need_event(vhost16_to_cpu(vq, event), new, old);
>  }
>  
>  /* This actually signals the guest, using eventfd. */
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a9cbbb1..78f3c5f 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -107,9 +107,6 @@ struct vhost_virtqueue {
>  	/* Last index we used. */
>  	u16 last_used_idx;
>  
> -	/* Last used evet we've seen */
> -	u16 last_used_event;
> -
>  	/* Used flags */
>  	u16 used_flags;
>  
> 
Seems very reasonable to me. Thanks Joe

Acked-by: Colin Ian King <colin.king at canonical.com>