From: Seth Jennings <sjenning@linux.vnet.ibm.com>
To: Weijie Yang <weijie.yang@samsung.com>
Cc: minchan@kernel.org, bob.liu@oracle.com, weijie.yang.kh@gmail.com,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] mm/zswap: bugfix: memory leak when re-swapon
Date: Mon, 9 Sep 2013 12:03:49 -0500 [thread overview]
Message-ID: <20130909170349.GD4701@variantweb.net> (raw)
In-Reply-To: <000901ceaac0$a5f28420$f1d78c60$%yang@samsung.com>
On Fri, Sep 06, 2013 at 01:16:45PM +0800, Weijie Yang wrote:
> zswap_tree is not freed when swapoff, and it got re-kmalloc in swapon,
> so memory-leak occurs.
>
> Modify: free memory of zswap_tree in zswap_frontswap_invalidate_area().
>
> Signed-off-by: Weijie Yang <weijie.yang@samsung.com>
> ---
> mm/zswap.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index deda2b6..cbd9578 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -816,6 +816,10 @@ static void zswap_frontswap_invalidate_area(unsigned type)
> }
> tree->rbroot = RB_ROOT;
> spin_unlock(&tree->lock);
> +
> + zbud_destroy_pool(tree->pool);
> + kfree(tree);
> + zswap_trees[type] = NULL;
You changed how this works from v1. Any particular reason?
In this version you free the tree structure, which is fine as long as we
know for sure nothing will try to access it afterward unless there is a
swapon to reactivate it.
I'm just a little worried about a race here between a store and
invalidate_area. I think there is probably some mechanism to prevent
this, I just haven't been able to demonstrate it to myself.
The situation I'm worried about is:
shrink_page_list()
add_to_swap() then return (gets the swap entry)
try_to_unmap() then return (sets the swap entry in the pte)
pageout()
swap_writepage()
zswap_frontswap_store()
interacting with a swapoff operation.
When zswap_frontswap_store() is called, we continue to hold the page
lock. I think that might block the loop in try_to_unuse(), called by
swapoff, until we release it after the store.
I think it should be fine. Just wanted to think it through.
Acked-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> }
>
> static struct zbud_ops zswap_zbud_ops = {
> --
> 1.7.10.4
>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2013-09-09 17:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-06 5:16 [PATCH v2 1/4] mm/zswap: bugfix: memory leak when re-swapon Weijie Yang
2013-09-06 6:31 ` Bob Liu
2013-09-09 17:03 ` Seth Jennings [this message]
2013-09-16 7:58 ` Weijie Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130909170349.GD4701@variantweb.net \
--to=sjenning@linux.vnet.ibm.com \
--cc=bob.liu@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=weijie.yang.kh@gmail.com \
--cc=weijie.yang@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).