Re: [PATCH net-next v2 1/4] mm: page_frag: Introduce page_frag_alloc_align()
From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: 2021-02-02 11:36:25
Also in:
linux-mm
On Sun, Jan 31, 2021 at 03:44:23PM +0800, Kevin Hao wrote:
In the current implementation of page_frag_alloc(), it doesn't have
any align guarantee for the returned buffer address. But for some
hardwares they do require the DMA buffer to be aligned correctly,
so we would have to use some workarounds like below if the buffers
allocated by the page_frag_alloc() are used by these hardwares for
DMA.
buf = page_frag_alloc(really_needed_size + align);
buf = PTR_ALIGN(buf, align);
These codes seems ugly and would waste a lot of memories if the buffers
are used in a network driver for the TX/RX.Isn't the memory wasted even with this change? I am not familiar with the frag allocator so I might be missing something, but from what I understood each page_frag_cache keeps only the offset inside the current page being allocated, offset which you ALIGN_DOWN() to match the alignment requirement. I don't see how that memory between the non-aligned and aligned offset is going to be used again before the entire page is freed.
quoted hunk ↗ jump to hunk
So introduce page_frag_alloc_align() to make sure that an aligned buffer address is returned. Signed-off-by: Kevin Hao <redacted> Acked-by: Vlastimil Babka <redacted> --- v2: - Inline page_frag_alloc() - Adopt Vlastimil's suggestion and add his Acked-by include/linux/gfp.h | 12 ++++++++++-- mm/page_alloc.c | 8 +++++--- 2 files changed, 15 insertions(+), 5 deletions(-)diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 6e479e9c48ce..39f4b3070d09 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h@@ -583,8 +583,16 @@ extern void free_pages(unsigned long addr, unsigned int order); struct page_frag_cache; extern void __page_frag_cache_drain(struct page *page, unsigned int count); -extern void *page_frag_alloc(struct page_frag_cache *nc, - unsigned int fragsz, gfp_t gfp_mask); +extern void *page_frag_alloc_align(struct page_frag_cache *nc, + unsigned int fragsz, gfp_t gfp_mask, + int align); + +static inline void *page_frag_alloc(struct page_frag_cache *nc, + unsigned int fragsz, gfp_t gfp_mask) +{ + return page_frag_alloc_align(nc, fragsz, gfp_mask, 0); +} + extern void page_frag_free(void *addr); #define __free_page(page) __free_pages((page), 0)diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 519a60d5b6f7..4667e7b6993b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c@@ -5137,8 +5137,8 @@ void __page_frag_cache_drain(struct page *page, unsigned int count) } EXPORT_SYMBOL(__page_frag_cache_drain); -void *page_frag_alloc(struct page_frag_cache *nc, - unsigned int fragsz, gfp_t gfp_mask) +void *page_frag_alloc_align(struct page_frag_cache *nc, + unsigned int fragsz, gfp_t gfp_mask, int align) { unsigned int size = PAGE_SIZE; struct page *page;@@ -5190,11 +5190,13 @@ void *page_frag_alloc(struct page_frag_cache *nc, } nc->pagecnt_bias--; + if (align) + offset = ALIGN_DOWN(offset, align); nc->offset = offset; return nc->va + offset; } -EXPORT_SYMBOL(page_frag_alloc); +EXPORT_SYMBOL(page_frag_alloc_align); /* * Frees a page fragment allocated out of either a compound or order 0 page.-- 2.29.2