* [PATCH] z3fold: fix memory leak in kmem cache
@ 2019-09-17 15:53 Vitaly Wool
2019-09-18 7:31 ` Vlastimil Babka
0 siblings, 1 reply; 4+ messages in thread
From: Vitaly Wool @ 2019-09-17 15:53 UTC (permalink / raw)
To: Linux-MM, Andrew Morton, linux-kernel
Cc: Dan Streetman, Vlastimil Babka, markus.linnala
Currently there is a leak in init_z3fold_page() -- it allocates
handles from kmem cache even for headless pages, but then they are
never used and never freed, so eventually kmem cache may get
exhausted. This patch provides a fix for that.
Reported-by: Markus Linnala <markus.linnala@gmail.com>
Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
---
mm/z3fold.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/mm/z3fold.c b/mm/z3fold.c
index 6397725b5ec6..7dffef2599c3 100644
--- a/mm/z3fold.c
+++ b/mm/z3fold.c
@@ -301,14 +301,11 @@ static void z3fold_unregister_migration(struct z3fold_pool *pool)
}
/* Initializes the z3fold header of a newly allocated z3fold page */
-static struct z3fold_header *init_z3fold_page(struct page *page,
+static struct z3fold_header *init_z3fold_page(struct page *page, bool headless,
struct z3fold_pool *pool, gfp_t gfp)
{
struct z3fold_header *zhdr = page_address(page);
- struct z3fold_buddy_slots *slots = alloc_slots(pool, gfp);
-
- if (!slots)
- return NULL;
+ struct z3fold_buddy_slots *slots;
INIT_LIST_HEAD(&page->lru);
clear_bit(PAGE_HEADLESS, &page->private);
@@ -316,6 +313,12 @@ static struct z3fold_header *init_z3fold_page(struct page *page,
clear_bit(NEEDS_COMPACTING, &page->private);
clear_bit(PAGE_STALE, &page->private);
clear_bit(PAGE_CLAIMED, &page->private);
+ if (headless)
+ return zhdr;
+
+ slots = alloc_slots(pool, gfp);
+ if (!slots)
+ return NULL;
spin_lock_init(&zhdr->page_lock);
kref_init(&zhdr->refcount);
@@ -962,7 +965,7 @@ static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp,
if (!page)
return -ENOMEM;
- zhdr = init_z3fold_page(page, pool, gfp);
+ zhdr = init_z3fold_page(page, bud == HEADLESS, pool, gfp);
if (!zhdr) {
__free_page(page);
return -ENOMEM;
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] z3fold: fix memory leak in kmem cache
2019-09-17 15:53 [PATCH] z3fold: fix memory leak in kmem cache Vitaly Wool
@ 2019-09-18 7:31 ` Vlastimil Babka
2019-09-19 6:47 ` Markus Linnala
2019-09-19 7:59 ` Vitaly Wool
0 siblings, 2 replies; 4+ messages in thread
From: Vlastimil Babka @ 2019-09-18 7:31 UTC (permalink / raw)
To: Vitaly Wool, Linux-MM, Andrew Morton, linux-kernel
Cc: Dan Streetman, markus.linnala
On 9/17/19 5:53 PM, Vitaly Wool wrote:
> Currently there is a leak in init_z3fold_page() -- it allocates
> handles from kmem cache even for headless pages, but then they are
> never used and never freed, so eventually kmem cache may get
> exhausted. This patch provides a fix for that.
>
> Reported-by: Markus Linnala <markus.linnala@gmail.com>
> Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
Can a Fixes: commit be pinpointed, and CC stable added?
> ---
> mm/z3fold.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/mm/z3fold.c b/mm/z3fold.c
> index 6397725b5ec6..7dffef2599c3 100644
> --- a/mm/z3fold.c
> +++ b/mm/z3fold.c
> @@ -301,14 +301,11 @@ static void z3fold_unregister_migration(struct z3fold_pool *pool)
> }
>
> /* Initializes the z3fold header of a newly allocated z3fold page */
> -static struct z3fold_header *init_z3fold_page(struct page *page,
> +static struct z3fold_header *init_z3fold_page(struct page *page, bool headless,
> struct z3fold_pool *pool, gfp_t gfp)
> {
> struct z3fold_header *zhdr = page_address(page);
> - struct z3fold_buddy_slots *slots = alloc_slots(pool, gfp);
> -
> - if (!slots)
> - return NULL;
> + struct z3fold_buddy_slots *slots;
>
> INIT_LIST_HEAD(&page->lru);
> clear_bit(PAGE_HEADLESS, &page->private);
> @@ -316,6 +313,12 @@ static struct z3fold_header *init_z3fold_page(struct page *page,
> clear_bit(NEEDS_COMPACTING, &page->private);
> clear_bit(PAGE_STALE, &page->private);
> clear_bit(PAGE_CLAIMED, &page->private);
> + if (headless)
> + return zhdr;
> +
> + slots = alloc_slots(pool, gfp);
> + if (!slots)
> + return NULL;
>
> spin_lock_init(&zhdr->page_lock);
> kref_init(&zhdr->refcount);
> @@ -962,7 +965,7 @@ static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp,
> if (!page)
> return -ENOMEM;
>
> - zhdr = init_z3fold_page(page, pool, gfp);
> + zhdr = init_z3fold_page(page, bud == HEADLESS, pool, gfp);
> if (!zhdr) {
> __free_page(page);
> return -ENOMEM;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] z3fold: fix memory leak in kmem cache
2019-09-18 7:31 ` Vlastimil Babka
@ 2019-09-19 6:47 ` Markus Linnala
2019-09-19 7:59 ` Vitaly Wool
1 sibling, 0 replies; 4+ messages in thread
From: Markus Linnala @ 2019-09-19 6:47 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Vitaly Wool, Linux-MM, Andrew Morton, linux-kernel, Dan Streetman
[-- Attachment #1: Type: text/plain, Size: 4052 bytes --]
ke 18. syysk. 2019 klo 10.35 Vlastimil Babka (vbabka@suse.cz) kirjoitti:
> On 9/17/19 5:53 PM, Vitaly Wool wrote:
> > Currently there is a leak in init_z3fold_page() -- it allocates
> > handles from kmem cache even for headless pages, but then they are
> > never used and never freed, so eventually kmem cache may get
> > exhausted. This patch provides a fix for that.
> >
> > Reported-by: Markus Linnala <markus.linnala@gmail.com>
> > Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
>
> Can a Fixes: commit be pinpointed, and CC stable added?
>
> > ---
> > mm/z3fold.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/z3fold.c b/mm/z3fold.c
> > index 6397725b5ec6..7dffef2599c3 100644
> > --- a/mm/z3fold.c
> > +++ b/mm/z3fold.c
> > @@ -301,14 +301,11 @@ static void z3fold_unregister_migration(struct
> z3fold_pool *pool)
> > }
> >
> > /* Initializes the z3fold header of a newly allocated z3fold page */
> > -static struct z3fold_header *init_z3fold_page(struct page *page,
> > +static struct z3fold_header *init_z3fold_page(struct page *page, bool
> headless,
> > struct z3fold_pool *pool, gfp_t
> gfp)
> > {
> > struct z3fold_header *zhdr = page_address(page);
> > - struct z3fold_buddy_slots *slots = alloc_slots(pool, gfp);
> > -
> > - if (!slots)
> > - return NULL;
> > + struct z3fold_buddy_slots *slots;
> >
> > INIT_LIST_HEAD(&page->lru);
> > clear_bit(PAGE_HEADLESS, &page->private);
> > @@ -316,6 +313,12 @@ static struct z3fold_header
> *init_z3fold_page(struct page *page,
> > clear_bit(NEEDS_COMPACTING, &page->private);
> > clear_bit(PAGE_STALE, &page->private);
> > clear_bit(PAGE_CLAIMED, &page->private);
> > + if (headless)
> > + return zhdr;
> > +
> > + slots = alloc_slots(pool, gfp);
> > + if (!slots)
> > + return NULL;
> >
> > spin_lock_init(&zhdr->page_lock);
> > kref_init(&zhdr->refcount);
> > @@ -962,7 +965,7 @@ static int z3fold_alloc(struct z3fold_pool *pool,
> size_t size, gfp_t gfp,
> > if (!page)
> > return -ENOMEM;
> >
> > - zhdr = init_z3fold_page(page, pool, gfp);
> > + zhdr = init_z3fold_page(page, bud == HEADLESS, pool, gfp);
> > if (!zhdr) {
> > __free_page(page);
> > return -ENOMEM;
> >
>
>
I have somwhat extensive test suite for this issue. Effectively:
for tmout in 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 30 30 30
30 900; do
stress --vm $(($(nproc)+2)) --vm-bytes $(($(awk '"'"'/MemAvail/{print
$2}'"'"' /proc/meminfo)*1024/$(nproc))) --timeout '"$tmout"
done
and then in another session run:
while true; do
bash -c '
declare -A arr;
b=();
for a in $(seq 7000);do
b+=($a);
arr["$a"]="${b[@]}";
done;
sleep 60;
'
sleep 20
done
This should make testing machine to have near constant memory pressure from
stress and then swapping and releasing swap from other script. And then
there is tons of stuff to manage virtual machine when it is stuck, update
kernels, collect logs and analyze logs.
I run tests in virtual machine (Fedora 30) with 4 vCPU 1 GiB memory.
There was still some issues with this patch. I ran my test suite about 72
hours and got 5 issues.
Vitaly send me patch with additional lines about page_claimed bit. After
running my test suite so far about 65 hours there has not been any issues.
When I first saw issues with zswap, I did git bisect run from v5.1 (good)
to v5.3-rc4 (bad) and got this:
commit 7c2b8baa61fe578af905342938ad12f8dbaeae79
Author: Vitaly Wool <...>
Date: Mon May 13 17:22:49 2019 -0700
mm/z3fold.c: add structure for buddy handles
For z3fold to be able to move its pages per request of the memory
subsystem, it should not use direct object addresses in handles.
Instead,
it will create abstract handles (3 per page) which will contain pointers
to z3fold objects. Thus, it will be possible to change these pointers
when z3fold page is moved.
[-- Attachment #2: Type: text/html, Size: 6009 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] z3fold: fix memory leak in kmem cache
2019-09-18 7:31 ` Vlastimil Babka
2019-09-19 6:47 ` Markus Linnala
@ 2019-09-19 7:59 ` Vitaly Wool
1 sibling, 0 replies; 4+ messages in thread
From: Vitaly Wool @ 2019-09-19 7:59 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Linux-MM, Andrew Morton, LKML, Dan Streetman, Markus Linnala, stable
On Wed, Sep 18, 2019 at 9:35 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/17/19 5:53 PM, Vitaly Wool wrote:
> > Currently there is a leak in init_z3fold_page() -- it allocates
> > handles from kmem cache even for headless pages, but then they are
> > never used and never freed, so eventually kmem cache may get
> > exhausted. This patch provides a fix for that.
> >
> > Reported-by: Markus Linnala <markus.linnala@gmail.com>
> > Signed-off-by: Vitaly Wool <vitalywool@gmail.com>
>
> Can a Fixes: commit be pinpointed, and CC stable added?
Fixes: 7c2b8baa61fe578 "mm/z3fold.c: add structure for buddy handles"
Best regards,
Vitaly
> > ---
> > mm/z3fold.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/z3fold.c b/mm/z3fold.c
> > index 6397725b5ec6..7dffef2599c3 100644
> > --- a/mm/z3fold.c
> > +++ b/mm/z3fold.c
> > @@ -301,14 +301,11 @@ static void z3fold_unregister_migration(struct z3fold_pool *pool)
> > }
> >
> > /* Initializes the z3fold header of a newly allocated z3fold page */
> > -static struct z3fold_header *init_z3fold_page(struct page *page,
> > +static struct z3fold_header *init_z3fold_page(struct page *page, bool headless,
> > struct z3fold_pool *pool, gfp_t gfp)
> > {
> > struct z3fold_header *zhdr = page_address(page);
> > - struct z3fold_buddy_slots *slots = alloc_slots(pool, gfp);
> > -
> > - if (!slots)
> > - return NULL;
> > + struct z3fold_buddy_slots *slots;
> >
> > INIT_LIST_HEAD(&page->lru);
> > clear_bit(PAGE_HEADLESS, &page->private);
> > @@ -316,6 +313,12 @@ static struct z3fold_header *init_z3fold_page(struct page *page,
> > clear_bit(NEEDS_COMPACTING, &page->private);
> > clear_bit(PAGE_STALE, &page->private);
> > clear_bit(PAGE_CLAIMED, &page->private);
> > + if (headless)
> > + return zhdr;
> > +
> > + slots = alloc_slots(pool, gfp);
> > + if (!slots)
> > + return NULL;
> >
> > spin_lock_init(&zhdr->page_lock);
> > kref_init(&zhdr->refcount);
> > @@ -962,7 +965,7 @@ static int z3fold_alloc(struct z3fold_pool *pool, size_t size, gfp_t gfp,
> > if (!page)
> > return -ENOMEM;
> >
> > - zhdr = init_z3fold_page(page, pool, gfp);
> > + zhdr = init_z3fold_page(page, bud == HEADLESS, pool, gfp);
> > if (!zhdr) {
> > __free_page(page);
> > return -ENOMEM;
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-19 7:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-17 15:53 [PATCH] z3fold: fix memory leak in kmem cache Vitaly Wool
2019-09-18 7:31 ` Vlastimil Babka
2019-09-19 6:47 ` Markus Linnala
2019-09-19 7:59 ` Vitaly Wool
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox