[PATCH] mm/page_alloc: fix the potential memory waste in page_frag_alloc_align

[PATCH] mm/page_alloc: fix the potential memory waste in page_frag_alloc_align

[wang wei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1443 bytes --]

First step, allocating a memory fragment with size 1KB bytes uses
page_frag_alloc_align.  It will allocate PAGE_FRAG_CACHE_MAX_SIZE
bytes by __page_frag_cache_refill, store the pointer at nc->va and
then return the last 1KB memory fragment which address is nc->va
+ PAGE_FRAG_CACHE_MAX_SIZE - 1KB. The remaining PAGE_FRAG_CACHE_MAX_SIZE
- 1KB bytes of memory can Meet future memory requests.

Second step, if the caller requests a memory fragment with size
more then PAGE_FRAG_CACHE_MAX_SIZE bytes,  page_frag_alloc_align,
it will also allocate PAGE_FRAG_CACHE_MAX_SIZE bytes by
__page_frag_cache_refill,  store the pointer at nc->va, and
return NULL.  this behavior makes the rest of
PAGE_FRAG_CACHE_MAX_SIZE - 1KB bytes memory at First step are
wasted(allocate from buddy system but not used).

So we should check the size of memory requests. If it beyound
PAGE_FRAG_CACHE_MAX_SIZE, we should return NULL directly.

Signed-off-by: wang wei <a929244872@163.com>
---
 mm/page_alloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8cf86d0c6aa8..3182c648e86e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4757,6 +4757,9 @@ void *page_frag_alloc_align(struct page_frag_cache *nc,
 	struct page *page;
 	int offset;
 
+	if（unlikely(fragsz > PAGE_FRAG_CACHE_MAX_SIZE)）
+		return NULL;
+
 	if (unlikely(!nc->va)) {
 refill:
 		page = __page_frag_cache_refill(nc, gfp_mask);
-- 
2.25.1

Re: [PATCH] mm/page_alloc: fix the potential memory waste in page_frag_alloc_align

[Matthew Wilcox]

On Mon, Oct 23, 2023 at 11:42:16PM +0800, wang wei wrote:
> First step, allocating a memory fragment with size 1KB bytes uses
> page_frag_alloc_align.  It will allocate PAGE_FRAG_CACHE_MAX_SIZE
> bytes by __page_frag_cache_refill, store the pointer at nc->va and
> then return the last 1KB memory fragment which address is nc->va
> + PAGE_FRAG_CACHE_MAX_SIZE - 1KB. The remaining PAGE_FRAG_CACHE_MAX_SIZE
> - 1KB bytes of memory can Meet future memory requests.
> 
> Second step, if the caller requests a memory fragment with size
> more then PAGE_FRAG_CACHE_MAX_SIZE bytes,  page_frag_alloc_align,
> it will also allocate PAGE_FRAG_CACHE_MAX_SIZE bytes by
> __page_frag_cache_refill,  store the pointer at nc->va, and
> return NULL.  this behavior makes the rest of
> PAGE_FRAG_CACHE_MAX_SIZE - 1KB bytes memory at First step are
> wasted(allocate from buddy system but not used).

We could do this, but have you ever seen this happen, or are you
just reading code and looking for problems?  If the latter, I think
you've misunderstood how this allocator is normally used.

Re:Re: [PATCH] mm/page_alloc: fix the potential memory waste in page_frag_alloc_align

[王伟]

At 2023-10-24 00:17:41, "Matthew Wilcox" <willy@infradead.org> wrote:
>On Mon, Oct 23, 2023 at 11:42:16PM +0800, wang wei wrote:
>> First step, allocating a memory fragment with size 1KB bytes uses
>> page_frag_alloc_align.  It will allocate PAGE_FRAG_CACHE_MAX_SIZE
>> bytes by __page_frag_cache_refill, store the pointer at nc->va and
>> then return the last 1KB memory fragment which address is nc->va
>> + PAGE_FRAG_CACHE_MAX_SIZE - 1KB. The remaining PAGE_FRAG_CACHE_MAX_SIZE
>> - 1KB bytes of memory can Meet future memory requests.
>> 
>> Second step, if the caller requests a memory fragment with size
>> more then PAGE_FRAG_CACHE_MAX_SIZE bytes,  page_frag_alloc_align,
>> it will also allocate PAGE_FRAG_CACHE_MAX_SIZE bytes by
>> __page_frag_cache_refill,  store the pointer at nc->va, and
>> return NULL.  this behavior makes the rest of
>> PAGE_FRAG_CACHE_MAX_SIZE - 1KB bytes memory at First step are
>> wasted(allocate from buddy system but not used).
>
>We could do this, but have you ever seen this happen, or are you
>just reading code and looking for problems?  If the latter, I think
>you've misunderstood how this allocator is normally used.

I am indeed reading the kernel mm-subsystem code recently. And I did 
two experiments in a ARM32 platform when i read page_frag_alloc_align code. 
In this platform, PAGE_FRAG_CACHE_MAX_SIZE = 32767(32K).

First experiment code:

static int __init test_ko_init(void)
{
	struct page_frag_cache nc;
	void *data1, *data2;

	data1 = __alloc_page_frag(&nc, 33 * 1024, NULL);
	printk("alloc 33KB data1 = %x, nc.va = %x \n", data1, nc.va);
	data2 = __alloc_page_frag(&nc, 33 * 1024, NULL);
	printk("alloc 33KB data2 = %x, nc.va = %x \n", data2, nc.va);
	return 0;
}

output:

[   19.939263] alloc 33KB data1 = 0, nc.va = c3930000
[   19.943790] alloc 33KB data2 = 0, nc.va = c3930000

In this experiment, i allocate two 33KB-size memory.  
__alloc_page_frag returns NULL indicates the memory request could not be meet.

Second experiment code:

static int __init test_ko_init(void)
{
	struct page_frag_cache nc;
	void *data1, *data2;

	data1 = __alloc_page_frag(&nc, 1 * 1024, NULL);
	printk("alloc 1KB data1 = %x, nc.va = %x \n", data1, nc.va);
	data2 = __alloc_page_frag(&nc, 33 * 1024, NULL);
	printk("alloc 33KB data2 = %x, nc.va = %x \n", data2, nc.va);
	return 0;
}

output:

[   15.906566] alloc 1KB data1 = c3937c00, nc.va = c3930000
[   15.911632] alloc 33KB data2 = 0, nc.va = c3938000

In this experiment, i allocate 1KB-size and 33KB-size memory.  
__alloc_page_frag returns NULL when i allocate 33KB-size memory.
But, nc.va is new pointer to a 32KB memory fragment. So the memory 
between c3930000 and c3937bff is unused. this what i said memory waste.

Based on the two experiments above, I suspect this will happen on 
other platforms as well. 

I am new commer for kernel code, and I am not very understand 
lost of kernel functions' use. Thanks for your advice.