[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <36366741-8df2-1137-0dd9-d498d0f770e4@huawei.com>
Date: Wed, 14 Jun 2023 11:51:51 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Alexander Duyck <alexander.duyck@...il.com>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Jesper Dangaard Brouer <hawk@...nel.org>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next v3 3/4] page_pool: introduce page_pool_alloc()
API
On 2023/6/13 22:36, Alexander Duyck wrote:
> On Fri, Jun 9, 2023 at 6:20 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
...
>>
>> +static inline struct page *page_pool_alloc(struct page_pool *pool,
>> + unsigned int *offset,
>> + unsigned int *size, gfp_t gfp)
>> +{
>> + unsigned int max_size = PAGE_SIZE << pool->p.order;
>> + struct page *page;
>> +
>> + *size = ALIGN(*size, dma_get_cache_alignment());
>> +
>> + if (WARN_ON(*size > max_size))
>> + return NULL;
>> +
>> + if ((*size << 1) > max_size || PAGE_POOL_DMA_USE_PP_FRAG_COUNT) {
>> + *size = max_size;
>> + *offset = 0;
>> + return page_pool_alloc_pages(pool, gfp);
>> + }
>> +
>> + page = __page_pool_alloc_frag(pool, offset, *size, gfp);
>> + if (unlikely(!page))
>> + return NULL;
>> +
>> + /* There is very likely not enough space for another frag, so append the
>> + * remaining size to the current frag to avoid truesize underestimate
>> + * problem.
>> + */
>> + if (pool->frag_offset + *size > max_size) {
>> + *size = max_size - *offset;
>> + pool->frag_offset = max_size;
>> + }
>> +
>
> Rather than preventing a truesize underestimation this will cause one.
> You are adding memory to the size of the page reserved and not
> accounting for it anywhere as this isn't reported up to the network
> stack. I would suggest dropping this from your patch.
I was thinking about the driver author reporting it up to the network
stack using the new API as something like below:
int truesize = size;
struct page *page;
int offset;
page = page_pool_dev_alloc(pool, &offset, &truesize);
if (unlikely(!page))
goto err;
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
offset, size, truesize);
and similiar handling for *_build_skb() case too.
Does it make senses for that? or did I miss something obvious here?
>
Powered by blists - more mailing lists