Commit 56ef27e3 authored by Alexander Lobakin's avatar Alexander Lobakin Committed by Jakub Kicinski
Browse files

page_pool: disable direct recycling based on pool->cpuid on destroy 



Now that direct recycling is performed basing on pool->cpuid when set,
memory leaks are possible:

1. A pool is destroyed.
2. Alloc cache is emptied (it's done only once).
3. pool->cpuid is still set.
4. napi_pp_put_page() does direct recycling basing on pool->cpuid.
5. Now alloc cache is not empty, but it won't ever be freed.

In order to avoid that, rewrite pool->cpuid to -1 when unlinking NAPI to
make sure no direct recycling will be possible after emptying the cache.
This involves a bit of overhead as pool->cpuid now must be accessed
via READ_ONCE() to avoid partial reads.
Rename page_pool_unlink_napi() -> page_pool_disable_direct_recycling()
to reflect what it actually does and unexport it.

Signed-off-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
Reviewed-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20240215113905.96817-1-aleksander.lobakin@intel.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 5983e5df

include/net/page_pool/types.h

+0 −5
Original line number Diff line number Diff line
@@ -210,17 +210,12 @@ struct page_pool *page_pool_create_percpu(const struct page_pool_params *params, 
struct xdp_mem_info;



#ifdef CONFIG_PAGE_POOL

void page_pool_unlink_napi(struct page_pool *pool);

void page_pool_destroy(struct page_pool *pool);

void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),

			   struct xdp_mem_info *mem);

void page_pool_put_page_bulk(struct page_pool *pool, void **data,

			     int count);

#else

static inline void page_pool_unlink_napi(struct page_pool *pool)

{

}



static inline void page_pool_destroy(struct page_pool *pool)

{

}

net/core/page_pool.c

+7 −3
Original line number Diff line number Diff line
@@ -949,8 +949,13 @@ void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *), 
	pool->xdp_mem_id = mem->id;

}



void page_pool_unlink_napi(struct page_pool *pool)

static void page_pool_disable_direct_recycling(struct page_pool *pool)

{

	/* Disable direct recycling based on pool->cpuid.

	 * Paired with READ_ONCE() in napi_pp_put_page().

	 */

	WRITE_ONCE(pool->cpuid, -1);



	if (!pool->p.napi)

		return;
@@ -962,7 +967,6 @@ void page_pool_unlink_napi(struct page_pool *pool) 


	WRITE_ONCE(pool->p.napi, NULL);

}

EXPORT_SYMBOL(page_pool_unlink_napi);



void page_pool_destroy(struct page_pool *pool)

{
@@ -972,7 +976,7 @@ void page_pool_destroy(struct page_pool *pool) 
	if (!page_pool_put(pool))

		return;



	page_pool_unlink_napi(pool);

	page_pool_disable_direct_recycling(pool);

	page_pool_free_frag(pool);



	if (!page_pool_release(pool))

net/core/skbuff.c

+1 −1
Original line number Diff line number Diff line
@@ -1018,7 +1018,7 @@ bool napi_pp_put_page(struct page *page, bool napi_safe) 
		unsigned int cpuid = smp_processor_id();



		allow_direct = napi && READ_ONCE(napi->list_owner) == cpuid;

		allow_direct |= (pp->cpuid == cpuid);

		allow_direct |= READ_ONCE(pp->cpuid) == cpuid;

	}



	/* Driver set this to memory recycling info. Reset it on recycle.