| Message ID | 20231023154216.376522-1-a929244872@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/page_alloc: fix the potential memory waste in page_frag_alloc_align | expand |
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.
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.
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);
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(+)