Re: [PATCH 1/3] vhost-net: check the support of mergeable bufferoutside the receive loop

[Michael S. Tsirkin]

On Mon, Jan 17, 2011 at 04:10:59PM +0800, Jason Wang wrote:
> No need to check the support of mergeable buffer inside the recevie
> loop as the whole handle_rx()_xx is in the read critical region.  So
> this patch move it ahead of the receiving loop.
> 
> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>

Well feature check is mostly just features & bit
so why would it be slower? Because of the rcu
adding memory barriers? Do you observe a
measureable speedup with this patch?

Apropos, I noticed that the check in vhost_has_feature
is wrong: it uses the same kind of RCU as the
private pointer. So we'll have to fix that properly
by adding more lockdep classes, but for now
we'll need to make
the check 1 || lockdep_is_held(&dev->mutex);
and add a TODO.

> ---
>  drivers/vhost/net.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 14fc189..95e49de 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -411,7 +411,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  	};
>  
>  	size_t total_len = 0;
> -	int err, headcount;
> +	int err, headcount, mergeable;
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock = rcu_dereference(vq->private_data);
> @@ -425,6 +425,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  
>  	vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) ?
>  		vq->log : NULL;
> +	mergeable = vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
>  
>  	while ((sock_len = peek_head_len(sock->sk))) {
>  		sock_len += sock_hlen;
> @@ -474,7 +475,7 @@ static void handle_rx_mergeable(struct vhost_net *net)
>  			break;
>  		}
>  		/* TODO: Should check and handle checksum. */
> -		if (vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF) &&
> +		if (likely(mergeable) &&
>  		    memcpy_toiovecend(vq->hdr, (unsigned char *)&headcount,
>  				      offsetof(typeof(hdr), num_buffers),
>  				      sizeof hdr.num_buffers)) {
--
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/