Re: [PATCH net-next v18 12/14] net: replace page_frag with page_frag_cache

[Paolo Abeni]

On 9/6/24 09:36, Yunsheng Lin wrote:
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 49811c9281d4..6190d9bfd618 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -953,7 +953,7 @@ static int __ip_append_data(struct sock *sk,
>   			    struct flowi4 *fl4,
>   			    struct sk_buff_head *queue,
>   			    struct inet_cork *cork,
> -			    struct page_frag *pfrag,
> +			    struct page_frag_cache *nc,
>   			    int getfrag(void *from, char *to, int offset,
>   					int len, int odd, struct sk_buff *skb),
>   			    void *from, int length, int transhdrlen,
> @@ -1228,13 +1228,19 @@ static int __ip_append_data(struct sock *sk,
>   			copy = err;
>   			wmem_alloc_delta += copy;
>   		} else if (!zc) {
> +			struct page_frag page_frag, *pfrag;
>   			int i = skb_shinfo(skb)->nr_frags;
> +			void *va;
>   
>   			err = -ENOMEM;
> -			if (!sk_page_frag_refill(sk, pfrag))
> +			pfrag = &page_frag;
> +			va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
> +			if (!va)
>   				goto error;
>   
>   			skb_zcopy_downgrade_managed(skb);
> +			copy = min_t(int, copy, pfrag->size);
> +
>   			if (!skb_can_coalesce(skb, i, pfrag->page,
>   					      pfrag->offset)) {
>   				err = -EMSGSIZE;
> @@ -1242,18 +1248,18 @@ static int __ip_append_data(struct sock *sk,
>   					goto error;
>   
>   				__skb_fill_page_desc(skb, i, pfrag->page,
> -						     pfrag->offset, 0);
> +						     pfrag->offset, copy);
>   				skb_shinfo(skb)->nr_frags = ++i;
> -				get_page(pfrag->page);
> +				page_frag_commit(nc, pfrag, copy);
> +			} else {
> +				skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1],
> +						  copy);
> +				page_frag_commit_noref(nc, pfrag, copy);
>   			}
> -			copy = min_t(int, copy, pfrag->size - pfrag->offset);
> -			if (getfrag(from,
> -				    page_address(pfrag->page) + pfrag->offset,
> -				    offset, copy, skb->len, skb) < 0)
> +
> +			if (getfrag(from, va, offset, copy, skb->len, skb) < 0)
>   				goto error_efault;

Should the 'commit' happen only when 'getfrag' is successful?

Similar question in the ipv6 code.

Thanks,

Paolo