Re: [PATCH 2/6] macvtap: zerocopy: fix truesize underestimation

[Michael S. Tsirkin]

On Mon, Apr 16, 2012 at 02:07:59PM +0800, Jason Wang wrote:
> As the skb fragment were pinned/built from user pages, we should
> account the page instead of length for truesize.
> 
> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>

I'm not sure this is right: the skb does *not* consume the
whole page, userspace uses the rest of the page
for other skbs. So we'll end up accounting for the
same page twice.
Eric, what's the right thing to do here in your opinion?

> ---
>  drivers/net/macvtap.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index bd4a70d..7cb2684 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -519,6 +519,7 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
>  		struct page *page[MAX_SKB_FRAGS];
>  		int num_pages;
>  		unsigned long base;
> +		unsigned long truesize;
>  
>  		len = from->iov_len - offset;
>  		if (!len) {
> @@ -533,10 +534,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
>  		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
>  			/* put_page is in skb free */
>  			return -EFAULT;
> +		truesize = size * PAGE_SIZE;
>  		skb->data_len += len;
>  		skb->len += len;
> -		skb->truesize += len;
> -		atomic_add(len, &skb->sk->sk_wmem_alloc);
> +		skb->truesize += truesize;
> +		atomic_add(truesize, &skb->sk->sk_wmem_alloc);
>  		while (len) {
>  			int off = base & ~PAGE_MASK;
>  			int size = min_t(int, len, PAGE_SIZE - off);
--
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/