On 2023/6/13 22:36, Alexander Duyck wrote:

On Fri, Jun 9, 2023 at 6:20 AM Yunsheng Lin [off-list ref] wrote:

...

quoted

+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?