* [PATCH v1] mm:free unused pages in kmalloc_order
@ 2020-06-27 4:55 Long Li
2020-06-29 2:42 ` Matthew Wilcox
2020-06-29 14:48 ` Christopher Lameter
0 siblings, 2 replies; 6+ messages in thread
From: Long Li @ 2020-06-27 4:55 UTC (permalink / raw)
To: cl, penberg, rientjes, iamjoonsoo.kim, akpm; +Cc: linux-mm, linux-kernel
Environment using the slub allocator, 1G memory in my ARM32.
kmalloc(1024, GFP_HIGHUSER) can allocate memory normally,
kmalloc(64*1024, GFP_HIGHUSER) will cause a memory leak, because
alloc_pages returns highmem physical pages, but it cannot be directly
converted into a virtual address and return NULL, the pages has not
been released. Usually driver developers will not use the
GFP_HIGHUSER flag to allocate memory in kmalloc, but I think this
memory leak is not perfect, it is best to be fixed. This is the
first time I have posted a patch, there may be something wrong.
Signed-off-by: Long Li <lonuxli.64@gmail.com>
---
mm/slab_common.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index a143a8c8f874..d2c53b980ab3 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -819,8 +819,12 @@ void *kmalloc_order(size_t size, gfp_t flags, unsigned int order)
page = alloc_pages(flags, order);
if (likely(page)) {
ret = page_address(page);
- mod_node_page_state(page_pgdat(page), NR_SLAB_UNRECLAIMABLE_B,
- PAGE_SIZE << order);
+ if (ret)
+ mod_node_page_state(page_pgdat(page),
+ NR_SLAB_UNRECLAIMABLE_B,
+ PAGE_SIZE << order);
+ else
+ __free_pages(page, order);
}
ret = kasan_kmalloc_large(ret, size, flags);
/* As ret might get tagged, call kmemleak hook after KASAN. */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1] mm:free unused pages in kmalloc_order
2020-06-27 4:55 [PATCH v1] mm:free unused pages in kmalloc_order Long Li
@ 2020-06-29 2:42 ` Matthew Wilcox
2020-06-29 14:49 ` Christopher Lameter
2020-06-29 14:48 ` Christopher Lameter
1 sibling, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2020-06-29 2:42 UTC (permalink / raw)
To: Long Li
Cc: cl, penberg, rientjes, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Sat, Jun 27, 2020 at 04:55:07AM +0000, Long Li wrote:
> Environment using the slub allocator, 1G memory in my ARM32.
> kmalloc(1024, GFP_HIGHUSER) can allocate memory normally,
> kmalloc(64*1024, GFP_HIGHUSER) will cause a memory leak, because
> alloc_pages returns highmem physical pages, but it cannot be directly
> converted into a virtual address and return NULL, the pages has not
> been released. Usually driver developers will not use the
> GFP_HIGHUSER flag to allocate memory in kmalloc, but I think this
> memory leak is not perfect, it is best to be fixed. This is the
> first time I have posted a patch, there may be something wrong.
Slab used to disallow GFP_HIGHMEM allocations earlier than this,
so you'd never get here. See one moron's patch here, 20 years ago ...
https://lore.kernel.org/lkml/20001228145801.C19693@parcelfarce.linux.theplanet.co.uk/
Things changed a bit since then. SLAB_LEVEL_MASK became GFP_SLAB_BUG_MASK,
then GFP_SLAB_BUG_MASK moved to mm/internal.h. Nowadays, GFP_SLAB_BUG_MASK
is checked only in new_slab(), and it is definitely skipped when we go through
the kmalloc_large() patch.
Could you send a replacement patch which checks GFP_SLAB_BUG_MASK before
calling alloc_pages()?
> +++ b/mm/slab_common.c
> @@ -819,8 +819,12 @@ void *kmalloc_order(size_t size, gfp_t flags, unsigned int order)
> page = alloc_pages(flags, order);
> if (likely(page)) {
> ret = page_address(page);
> - mod_node_page_state(page_pgdat(page), NR_SLAB_UNRECLAIMABLE_B,
> - PAGE_SIZE << order);
> + if (ret)
> + mod_node_page_state(page_pgdat(page),
> + NR_SLAB_UNRECLAIMABLE_B,
> + PAGE_SIZE << order);
> + else
> + __free_pages(page, order);
> }
> ret = kasan_kmalloc_large(ret, size, flags);
> /* As ret might get tagged, call kmemleak hook after KASAN. */
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] mm:free unused pages in kmalloc_order
2020-06-29 2:42 ` Matthew Wilcox
@ 2020-06-29 14:49 ` Christopher Lameter
0 siblings, 0 replies; 6+ messages in thread
From: Christopher Lameter @ 2020-06-29 14:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Long Li, penberg, rientjes, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Mon, 29 Jun 2020, Matthew Wilcox wrote:
> Slab used to disallow GFP_HIGHMEM allocations earlier than this,
It is still not allowed and not supported.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] mm:free unused pages in kmalloc_order
2020-06-27 4:55 [PATCH v1] mm:free unused pages in kmalloc_order Long Li
2020-06-29 2:42 ` Matthew Wilcox
@ 2020-06-29 14:48 ` Christopher Lameter
2020-06-29 14:52 ` Matthew Wilcox
1 sibling, 1 reply; 6+ messages in thread
From: Christopher Lameter @ 2020-06-29 14:48 UTC (permalink / raw)
To: Long Li; +Cc: penberg, rientjes, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Sat, 27 Jun 2020, Long Li wrote:
> Environment using the slub allocator, 1G memory in my ARM32.
> kmalloc(1024, GFP_HIGHUSER) can allocate memory normally,
> kmalloc(64*1024, GFP_HIGHUSER) will cause a memory leak, because
> alloc_pages returns highmem physical pages, but it cannot be directly
> converted into a virtual address and return NULL, the pages has not
> been released. Usually driver developers will not use the
> GFP_HIGHUSER flag to allocate memory in kmalloc, but I think this
> memory leak is not perfect, it is best to be fixed. This is the
> first time I have posted a patch, there may be something wrong.
Highmem is not supported by the slab allocators. Please ensure that there
is a warning generated if someone attempts to do such an allocation. We
used to check for that.
In order to make such allocations possible one would have to create yet
another kmalloc array for high memory.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] mm:free unused pages in kmalloc_order
2020-06-29 14:48 ` Christopher Lameter
@ 2020-06-29 14:52 ` Matthew Wilcox
2020-07-01 15:18 ` Christopher Lameter
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2020-06-29 14:52 UTC (permalink / raw)
To: Christopher Lameter
Cc: Long Li, penberg, rientjes, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Mon, Jun 29, 2020 at 02:48:06PM +0000, Christopher Lameter wrote:
> On Sat, 27 Jun 2020, Long Li wrote:
> > Environment using the slub allocator, 1G memory in my ARM32.
> > kmalloc(1024, GFP_HIGHUSER) can allocate memory normally,
> > kmalloc(64*1024, GFP_HIGHUSER) will cause a memory leak, because
> > alloc_pages returns highmem physical pages, but it cannot be directly
> > converted into a virtual address and return NULL, the pages has not
> > been released. Usually driver developers will not use the
> > GFP_HIGHUSER flag to allocate memory in kmalloc, but I think this
> > memory leak is not perfect, it is best to be fixed. This is the
> > first time I have posted a patch, there may be something wrong.
>
> Highmem is not supported by the slab allocators. Please ensure that there
> is a warning generated if someone attempts to do such an allocation. We
> used to check for that.
Sounds like we need a test somewhere that checks this behaviour.
> In order to make such allocations possible one would have to create yet
> another kmalloc array for high memory.
Not for this case because it goes straight to kmalloc_order(). What does
make this particular case impossible is that we can't kmap() a compound
page. We could vmap it, but why are we bothering?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] mm:free unused pages in kmalloc_order
2020-06-29 14:52 ` Matthew Wilcox
@ 2020-07-01 15:18 ` Christopher Lameter
0 siblings, 0 replies; 6+ messages in thread
From: Christopher Lameter @ 2020-07-01 15:18 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Long Li, penberg, rientjes, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Mon, 29 Jun 2020, Matthew Wilcox wrote:
> Sounds like we need a test somewhere that checks this behaviour.
>
> > In order to make such allocations possible one would have to create yet
> > another kmalloc array for high memory.
>
> Not for this case because it goes straight to kmalloc_order(). What does
> make this particular case impossible is that we can't kmap() a compound
> page. We could vmap it, but why are we bothering?
Well yes it will work if the slab allocator falls back to the page
allocator. Higher order allocation through kmalloc ;-). How much fun
and uselessness ....
Why not call the page allocator directly and play with all the bits you
want? Any regular small object allocation with GFP_HIGH will lead to
strange effects if the bit is not checked.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-01 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-27 4:55 [PATCH v1] mm:free unused pages in kmalloc_order Long Li
2020-06-29 2:42 ` Matthew Wilcox
2020-06-29 14:49 ` Christopher Lameter
2020-06-29 14:48 ` Christopher Lameter
2020-06-29 14:52 ` Matthew Wilcox
2020-07-01 15:18 ` Christopher Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).