diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c index b723ff5e5249cfbcf1a1c4c4a6f6597360b9a1c5..2ef77918009959c2911c3482ef1059b9c1c45c01 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c @@ -662,8 +662,8 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq, xdpi = mlx5e_xdpi_fifo_pop(xdpi_fifo); page = xdpi.page.page; - /* No need to check ((page->pp_magic & ~0x3UL) == PP_SIGNATURE) - * as we know this is a page_pool page. + /* No need to check page_pool_page_is_pp() as we + * know this is a page_pool page. */ page_pool_recycle_direct(page->pp, page); } while (++n < num); diff --git a/include/linux/mm.h b/include/linux/mm.h index 8999dcf606fa2d62e35a8a75c564c1819c2c3518..86909d01bf2cb02025cb737fdf8f72b242141413 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -4355,4 +4355,72 @@ static inline bool vma_is_peer_shared(struct vm_area_struct *vma) } #endif +/* + * DMA mapping IDs for page_pool + * + * When DMA-mapping a page, page_pool allocates an ID (from an xarray) and + * stashes it in the upper bits of page->pp_magic. We always want to be able to + * unambiguously identify page pool pages (using page_pool_page_is_pp()). Non-PP + * pages can have arbitrary kernel pointers stored in the same field as pp_magic + * (since it overlaps with page->lru.next), so we must ensure that we cannot + * mistake a valid kernel pointer with any of the values we write into this + * field. + * + * On architectures that set POISON_POINTER_DELTA, this is already ensured, + * since this value becomes part of PP_SIGNATURE; meaning we can just use the + * space between the PP_SIGNATURE value (without POISON_POINTER_DELTA), and the + * lowest bits of POISON_POINTER_DELTA. On arches where POISON_POINTER_DELTA is + * 0, we use the lowest bit of PAGE_OFFSET as the boundary if that value is + * known at compile-time. + * + * If the value of PAGE_OFFSET is not known at compile time, or if it is too + * small to leave at least 8 bits available above PP_SIGNATURE, we define the + * number of bits to be 0, which turns off the DMA index tracking altogether + * (see page_pool_register_dma_index()). + */ +#define PP_DMA_INDEX_SHIFT (1 + __fls(PP_SIGNATURE - POISON_POINTER_DELTA)) + +/* Hardcode the MIN macro in the below commit: + * 1a251f52cfdc ("minmax: make generic MIN() and MAX() macros available everywhere") + */ +#define PP_MIN(x, y) ((x) < (y) ? (x) : (y)) + +#if POISON_POINTER_DELTA > 0 +/* PP_SIGNATURE includes POISON_POINTER_DELTA, so limit the size of the DMA + * index to not overlap with that if set + */ +#define PP_DMA_INDEX_BITS PP_MIN(32, __ffs(POISON_POINTER_DELTA) - PP_DMA_INDEX_SHIFT) +#else +/* Use the lowest bit of PAGE_OFFSET if there's at least 8 bits available; see above */ +#define PP_DMA_INDEX_MIN_OFFSET (1 << (PP_DMA_INDEX_SHIFT + 8)) +#define PP_DMA_INDEX_BITS ((__builtin_constant_p(PAGE_OFFSET) && \ + PAGE_OFFSET >= PP_DMA_INDEX_MIN_OFFSET && \ + !(PAGE_OFFSET & (PP_DMA_INDEX_MIN_OFFSET - 1))) ? \ + PP_MIN(32, __ffs(PAGE_OFFSET) - PP_DMA_INDEX_SHIFT) : 0) + +#endif + +#define PP_DMA_INDEX_MASK GENMASK(PP_DMA_INDEX_BITS + PP_DMA_INDEX_SHIFT - 1, \ + PP_DMA_INDEX_SHIFT) + +/* Mask used for checking in page_pool_page_is_pp() below. page->pp_magic is + * OR'ed with PP_SIGNATURE after the allocation in order to preserve bit 0 for + * the head page of compound page and bit 1 for pfmemalloc page, as well as the + * bits used for the DMA index. page_is_pfmemalloc() is checked in + * __page_pool_put_page() to avoid recycling the pfmemalloc page. + */ +#define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL) + +#ifdef CONFIG_PAGE_POOL +static inline bool page_pool_page_is_pp(struct page *page) +{ + return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE; +} +#else +static inline bool page_pool_page_is_pp(struct page *page) +{ + return false; +} +#endif + #endif /* _LINUX_MM_H */ diff --git a/include/linux/poison.h b/include/linux/poison.h index 851a855d386884177eb50db59a67821168c2258d..96f09b600af266bbb0ccc8b09804ea2d4fbf72ff 100644 --- a/include/linux/poison.h +++ b/include/linux/poison.h @@ -79,6 +79,10 @@ #define KEY_DESTROY 0xbd /********** net/core/page_pool.c **********/ +/* + * page_pool uses additional free bits within this value to store data, see the + * definition of PP_DMA_INDEX_MASK in mm.h + */ #define PP_SIGNATURE (0x40 + POISON_POINTER_DELTA) /********** net/core/skbuff.c **********/ diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index b5b6d0438c3822abf0f9843e602b0a87a34607bf..d6e171e79009e49dc2687b01d5bfcb1b59892e31 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -6,6 +6,7 @@ #include #include #include +#include #define PP_FLAG_DMA_MAP BIT(0) /* Should page_pool do the DMA * map/unmap @@ -23,6 +24,9 @@ PP_FLAG_DMA_SYNC_DEV |\ PP_FLAG_PAGE_FRAG) +/* Index limit to stay within PP_DMA_INDEX_BITS for DMA indices */ +#define PP_DMA_INDEX_LIMIT XA_LIMIT(1, BIT(PP_DMA_INDEX_BITS) - 1) + /* * Fast allocation side cache array/stack * @@ -182,7 +186,7 @@ struct page_pool { u64 destroy_cnt; - KABI_RESERVE(1) + KABI_USE(1, struct xarray *dma_mapped) KABI_RESERVE(2) }; diff --git a/mm/page_alloc.c b/mm/page_alloc.c index ca6c8d356c981ef0fe29e5c572e70d5978a2e2b2..a11cf7843bf69b3ed6f56b30ff4095c8da750724 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -878,6 +878,7 @@ static inline bool page_expected_state(struct page *page, #ifdef CONFIG_MEMCG page->memcg_data | #endif + page_pool_page_is_pp(page) | (page->flags & check_flags))) return false; @@ -904,6 +905,8 @@ static const char *page_bad_reason(struct page *page, unsigned long flags) if (unlikely(page->memcg_data)) bad_reason = "page still charged to cgroup"; #endif + if (unlikely(page_pool_page_is_pp(page))) + bad_reason = "page_pool leak"; return bad_reason; } diff --git a/net/core/page_pool.c b/net/core/page_pool.c index cb7238043a339b77f42d73389a89ee39c4faab47..22a01281ef519b1808807b2369344d17b3a93180 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -233,8 +233,17 @@ static int page_pool_init(struct page_pool *pool, /* Driver calling page_pool_create() also call page_pool_destroy() */ refcount_set(&pool->user_cnt, 1); - if (pool->p.flags & PP_FLAG_DMA_MAP) - get_device(pool->p.dev); + pool->dma_mapped = kmalloc(sizeof(*pool->dma_mapped), GFP_KERNEL); + if (!pool->dma_mapped) { +#ifdef CONFIG_PAGE_POOL_STATS + free_percpu(pool->recycle_stats); +#endif + ptr_ring_cleanup(&pool->ring, NULL); + + return -ENOMEM; + } + + xa_init_flags(pool->dma_mapped, XA_FLAGS_ALLOC1); return 0; } @@ -347,9 +356,70 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool, pool->p.dma_dir); } -static bool page_pool_dma_map(struct page_pool *pool, struct page *page) +static unsigned long page_get_dma_index(struct page *page) +{ + return (page->pp_magic & PP_DMA_INDEX_MASK) >> PP_DMA_INDEX_SHIFT; +} + +static void page_set_dma_index(struct page *page, unsigned long id) +{ + unsigned long magic; + + magic = page->pp_magic | (id << PP_DMA_INDEX_SHIFT); + page->pp_magic = magic; +} + +static int page_pool_register_dma_index(struct page_pool *pool, + struct page *page, gfp_t gfp) +{ + int err = 0; + u32 id; + + if (unlikely(!PP_DMA_INDEX_BITS)) + goto out; + + if (in_softirq()) + err = xa_alloc(pool->dma_mapped, &id, page, PP_DMA_INDEX_LIMIT, gfp); + else + err = xa_alloc_bh(pool->dma_mapped, &id, page, PP_DMA_INDEX_LIMIT, gfp); + if (err) { + WARN_ONCE(err != -ENOMEM, "couldn't track DMA mapping, please report to netdev@"); + goto out; + } + + page_set_dma_index(page, id); +out: + return err; +} + +static int page_pool_release_dma_index(struct page_pool *pool, + struct page *page) +{ + struct page *old; + unsigned long id; + + if (unlikely(!PP_DMA_INDEX_BITS)) + return 0; + + id = page_get_dma_index(page); + if (!id) + return -1; + + if (in_softirq()) + old = xa_cmpxchg(pool->dma_mapped, id, page, NULL, 0); + else + old = xa_cmpxchg_bh(pool->dma_mapped, id, page, NULL, 0); + if (old != page) + return -1; + + page_set_dma_index(page, 0); + return 0; +} + +static bool page_pool_dma_map(struct page_pool *pool, struct page *page, gfp_t gfp) { dma_addr_t dma; + int err; /* Setup DMA mapping: use 'struct page' area for storing DMA-addr * since dma_addr_t can be either 32 or 64 bits and does not always fit @@ -363,6 +433,14 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page) if (dma_mapping_error(pool->p.dev, dma)) return false; + err = page_pool_register_dma_index(pool, page, gfp); + if (err) { + dma_unmap_page_attrs(pool->p.dev, dma, PAGE_SIZE << pool->p.order, + pool->p.dma_dir, + DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING); + return false; + } + page_pool_set_dma_addr(page, dma); if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) @@ -397,7 +475,7 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool, return NULL; if ((pool->p.flags & PP_FLAG_DMA_MAP) && - unlikely(!page_pool_dma_map(pool, page))) { + unlikely(!page_pool_dma_map(pool, page, gfp))) { put_page(page); return NULL; } @@ -444,7 +522,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool, for (i = 0; i < nr_pages; i++) { page = pool->alloc.cache[i]; if ((pp_flags & PP_FLAG_DMA_MAP) && - unlikely(!page_pool_dma_map(pool, page))) { + unlikely(!page_pool_dma_map(pool, page, gfp))) { put_page(page); continue; } @@ -506,21 +584,19 @@ static s32 page_pool_inflight(struct page_pool *pool) return inflight; } -/* Disconnects a page (from a page_pool). API users can have a need - * to disconnect a page (from a page_pool), to allow it to be used as - * a regular page (that will eventually be returned to the normal - * page-allocator via put_page). - */ -static void page_pool_return_page(struct page_pool *pool, struct page *page) +static __always_inline +void __page_pool_release_page_dma(struct page_pool *pool, struct page *page) { dma_addr_t dma; - int count; if (!(pool->p.flags & PP_FLAG_DMA_MAP)) /* Always account for inflight pages, even if we didn't * map them */ - goto skip_dma_unmap; + return; + + if (page_pool_release_dma_index(pool, page)) + return; dma = page_pool_get_dma_addr(page); @@ -529,7 +605,19 @@ static void page_pool_return_page(struct page_pool *pool, struct page *page) PAGE_SIZE << pool->p.order, pool->p.dma_dir, DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING); page_pool_set_dma_addr(page, 0); -skip_dma_unmap: +} + +/* Disconnects a page (from a page_pool). API users can have a need + * to disconnect a page (from a page_pool), to allow it to be used as + * a regular page (that will eventually be returned to the normal + * page-allocator via put_page). + */ +void page_pool_return_page(struct page_pool *pool, struct page *page) +{ + int count; + + __page_pool_release_page_dma(pool, page); + page_pool_clear_pp_info(page); /* This may be the last page returned, releasing the pool, so @@ -602,9 +690,13 @@ __page_pool_put_page(struct page_pool *pool, struct page *page, if (likely(page_ref_count(page) == 1 && !page_is_pfmemalloc(page))) { /* Read barrier done in page_ref_count / READ_ONCE */ - if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) + if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) { + /* re-check under rcu_read_lock() to sync with page_pool_scrub() */ + rcu_read_lock(); page_pool_dma_sync_for_device(pool, page, dma_sync_size); + rcu_read_unlock(); + } if (allow_direct && in_softirq() && page_pool_recycle_in_cache(page, pool)) @@ -806,8 +898,8 @@ static void page_pool_free(struct page_pool *pool) ptr_ring_cleanup(&pool->ring, NULL); - if (pool->p.flags & PP_FLAG_DMA_MAP) - put_device(pool->p.dev); + xa_destroy(pool->dma_mapped); + kfree(pool->dma_mapped); #ifdef CONFIG_PAGE_POOL_STATS free_percpu(pool->recycle_stats); @@ -834,8 +926,28 @@ static void page_pool_empty_alloc_cache_once(struct page_pool *pool) static void page_pool_scrub(struct page_pool *pool) { + unsigned long id; + void *ptr; + page_pool_empty_alloc_cache_once(pool); - pool->destroy_cnt++; + if (!pool->destroy_cnt++ && pool->p.flags & PP_FLAG_DMA_MAP) { + if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV) { + /* Disable page_pool_dma_sync_for_device() */ + pool->p.flags &= ~PP_FLAG_DMA_SYNC_DEV; + + /* Make sure all concurrent returns that may see the old + * value of dma_sync (and thus perform a sync) have + * finished before doing the unmapping below. Skip the + * wait if the device doesn't actually need syncing, or + * if there are no outstanding mapped pages. + */ + if (!xa_empty(pool->dma_mapped)) + synchronize_net(); + } + + xa_for_each(pool->dma_mapped, id, ptr) + __page_pool_release_page_dma(pool, ptr); + } /* No more consumers should exist, but producers could still * be in-flight. diff --git a/net/core/skbuff.c b/net/core/skbuff.c index ea5e9d46d4c42e4e4af52b20ce6158e512725ebe..1d5958c1aeedb885d0eed81aafb316fe48ec198d 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -899,14 +899,7 @@ bool napi_pp_put_page(struct page *page, bool napi_safe) page = compound_head(page); - /* page->pp_magic is OR'ed with PP_SIGNATURE after the allocation - * in order to preserve any existing bits, such as bit 0 for the - * head page of compound page and bit 1 for pfmemalloc page, so - * mask those bits for freeing side when doing below checking, - * and page_is_pfmemalloc() is checked in __page_pool_put_page() - * to avoid recycling the pfmemalloc page. - */ - if (unlikely((page->pp_magic & ~0x3UL) != PP_SIGNATURE)) + if (unlikely(!page_pool_page_is_pp(page))) return false; pp = page->pp; diff --git a/net/core/xdp.c b/net/core/xdp.c index 5ee3f8f165e5aa68bcc1134087a25956ea685a33..d816456329c9613394d89b8506dffce67a71ca6c 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -380,8 +380,8 @@ void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct, page = virt_to_head_page(data); if (napi_direct && xdp_return_frame_no_direct()) napi_direct = false; - /* No need to check ((page->pp_magic & ~0x3UL) == PP_SIGNATURE) - * as mem->type knows this a page_pool page + /* No need to check page_pool_page_is_pp() as mem->type + * knows this a page_pool page */ page_pool_put_full_page(page->pp, page, napi_direct); break;