Re: [PATCH] vhost-net: switch to smp barriers

[Michael S. Tsirkin]

On Mon, Feb 01, 2010 at 07:21:02PM +0200, Michael S. Tsirkin wrote:
> vhost-net only uses memory barriers to control SMP effects
> (communication with userspace potentially running on a different CPU),
> so it should use SMP barriers and not mandatory barriers for memory
> access ordering, as suggested by Documentation/memory-barriers.txt
> 
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>


Rusty, any feedback on this one?
Thanks!

> ---
>  drivers/vhost/vhost.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> The above applies on top of net-next-2.6. Does not seem to give any
> measureable performance gain, but seems to generate less code
> and I think it's better to use correct APIs.
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index c8c25db..6eb1525 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -685,7 +685,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
>  	int i, r;
>  
>  	/* Make sure data written is seen before log. */
> -	wmb();
> +	smp_wmb();
>  	for (i = 0; i < log_num; ++i) {
>  		u64 l = min(log[i].len, len);
>  		r = log_write(vq->log_base, log[i].addr, l);
> @@ -884,7 +884,7 @@ unsigned vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq,
>  		return vq->num;
>  
>  	/* Only get avail ring entries after they have been exposed by guest. */
> -	rmb();
> +	smp_rmb();
>  
>  	/* Grab the next descriptor number they're advertising, and increment
>  	 * the index we've seen. */
> @@ -996,14 +996,14 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
>  		return -EFAULT;
>  	}
>  	/* Make sure buffer is written before we update index. */
> -	wmb();
> +	smp_wmb();
>  	if (put_user(vq->last_used_idx + 1, &vq->used->idx)) {
>  		vq_err(vq, "Failed to increment used idx");
>  		return -EFAULT;
>  	}
>  	if (unlikely(vq->log_used)) {
>  		/* Make sure data is seen before log. */
> -		wmb();
> +		smp_wmb();
>  		log_write(vq->log_base, vq->log_addr + sizeof *vq->used->ring *
>  			  (vq->last_used_idx % vq->num),
>  			  sizeof *vq->used->ring);
> @@ -1060,7 +1060,7 @@ bool vhost_enable_notify(struct vhost_virtqueue *vq)
>  	}
>  	/* They could have slipped one in as we were doing that: make
>  	 * sure it's written, then check again. */
> -	mb();
> +	smp_mb();
>  	r = get_user(avail_idx, &vq->avail->idx);
>  	if (r) {
>  		vq_err(vq, "Failed to check avail idx at %p: %d\n",
> -- 
> 1.6.6.144.g5c3af
--
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/