On 2021/7/20 23:43, Alexander Duyck wrote:

On Mon, Jul 19, 2021 at 8:36 PM Yunsheng Lin [off-list ref] wrote:

quoted

For 32 bit systems with 64 bit dma, dma_addr[1] is used to
store the upper 32 bit dma addr, those system should be rare
those days.

For normal system, the dma_addr[1] in 'struct page' is not
used, so we can reuse dma_addr[1] for storing frag count,
which means how many frags this page might be splited to.

In order to simplify the page frag support in the page pool,
the PAGE_POOL_DMA_USE_PP_FRAG_COUNT macro is added to indicate
the 32 bit systems with 64 bit dma, and the page frag support
in page pool is disabled for such system.

The newly added page_pool_set_frag_count() is called to reserve
the maximum frag count before any page frag is passed to the
user. The page_pool_atomic_sub_frag_count_return() is called
when user is done with the page frag.

Signed-off-by: Yunsheng Lin <redacted>
---
 include/linux/mm_types.h | 18 +++++++++++++-----
 include/net/page_pool.h  | 41 ++++++++++++++++++++++++++++++++++-------
 net/core/page_pool.c     |  4 ++++
 3 files changed, 51 insertions(+), 12 deletions(-)

<snip>

quoted

+static inline long page_pool_atomic_sub_frag_count_return(struct page *page,
+                                                         long nr)
+{
+       long frag_count = atomic_long_read(&page->pp_frag_count);
+       long ret;
+
+       if (frag_count == nr)
+               return 0;
+
+       ret = atomic_long_sub_return(nr, &page->pp_frag_count);
+       WARN_ON(ret < 0);
+       return ret;
 }

So this should just be an atomic_long_sub_return call. You should get
rid of the atomic_long_read portion of this as it can cover up
reference count errors.

atomic_long_sub_return() is used to avoid one possible cache bouncing and
barrrier caused by the last user.

You are right that that may cover up the reference count errors. How about
something like below:

static inline long page_pool_atomic_sub_frag_count_return(struct page *page,
							  long nr)
{
#ifdef CONFIG_DEBUG_PAGE_REF
	long ret = atomic_long_sub_return(nr, &page->pp_frag_count);

	WARN_ON(ret < 0);

	return ret;
#else
	if (atomic_long_read(&page->pp_frag_count) == nr)
		return 0;

	return atomic_long_sub_return(nr, &page->pp_frag_count);
#end
}

Or any better suggestion?

.