From: Hugh Dickins <hughd@google.com> To: "Huang, Ying" <ying.huang@intel.com> Cc: Hugh Dickins <hughd@google.com>, Tim Chen <tim.c.chen@linux.intel.com>, Minchan Kim <minchan@kernel.org>, Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: swap_cluster_info lockdep splat Date: Fri, 17 Feb 2017 10:42:37 -0800 (PST) [thread overview] Message-ID: <alpine.LSU.2.11.1702171036010.1638@eggly.anvils> (raw) In-Reply-To: <874lzt6znd.fsf@yhuang-dev.intel.com> On Fri, 17 Feb 2017, Huang, Ying wrote: > > I found a memory leak in __read_swap_cache_async() introduced by mm-swap > series, and confirmed it via testing. Could you verify whether it fixed > your cases? Thanks a lot for reporting. Well caught! That indeed fixes the leak I've been seeing: my load has now passed the 7 hour danger mark, with no indication of slowing down. I'll keep it running until I need to try something else on that machine, but all good for now. You could add Tested-by: Hugh Dickins <hughd@google.com> but don't bother: I'm sure Andrew will simply fold this fix into the fixed patch later on. Thanks, Hugh > > Best Regards, > Huang, Ying > > -------------------------------------------------------------------------> > From 4b96423796ab7435104eb2cb4dcf5d525b9e0800 Mon Sep 17 00:00:00 2001 > From: Huang Ying <ying.huang@intel.com> > Date: Fri, 17 Feb 2017 10:31:37 +0800 > Subject: [PATCH] mm, swap: Fix memory leak in __read_swap_cache_async() > > The memory may be leaked in __read_swap_cache_async(). For the cases > as below, > > CPU 0 CPU 1 > ----- ----- > > find_get_page() == NULL > __swp_swapcount() != 0 > new_page = alloc_page_vma() > radix_tree_maybe_preload() > swap in swap slot > swapcache_prepare() == -EEXIST > cond_resched() > reclaim the swap slot > find_get_page() == NULL > __swp_swapcount() == 0 > return NULL <- new_page leaked here !!! > > The memory leak has been confirmed via checking the value of new_page > when returning inside the loop in __read_swap_cache_async(). > > This is fixed via replacing return with break inside of loop in > __read_swap_cache_async(), so that there is opportunity for the > new_page to be checked and freed. > > Reported-by: Hugh Dickins <hughd@google.com> > Cc: Tim Chen <tim.c.chen@linux.intel.com> > Signed-off-by: "Huang, Ying" <ying.huang@intel.com> > --- > mm/swap_state.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 2126e9ba23b2..473b71e052a8 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -333,7 +333,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, > * else swap_off will be aborted if we return NULL. > */ > if (!__swp_swapcount(entry) && swap_slot_cache_enabled) > - return NULL; > + break; > > /* > * Get a new page to read into from swap. > -- > 2.11.0 > >
WARNING: multiple messages have this Message-ID (diff)
From: Hugh Dickins <hughd@google.com> To: "Huang, Ying" <ying.huang@intel.com> Cc: Hugh Dickins <hughd@google.com>, Tim Chen <tim.c.chen@linux.intel.com>, Minchan Kim <minchan@kernel.org>, Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: swap_cluster_info lockdep splat Date: Fri, 17 Feb 2017 10:42:37 -0800 (PST) [thread overview] Message-ID: <alpine.LSU.2.11.1702171036010.1638@eggly.anvils> (raw) In-Reply-To: <874lzt6znd.fsf@yhuang-dev.intel.com> On Fri, 17 Feb 2017, Huang, Ying wrote: > > I found a memory leak in __read_swap_cache_async() introduced by mm-swap > series, and confirmed it via testing. Could you verify whether it fixed > your cases? Thanks a lot for reporting. Well caught! That indeed fixes the leak I've been seeing: my load has now passed the 7 hour danger mark, with no indication of slowing down. I'll keep it running until I need to try something else on that machine, but all good for now. You could add Tested-by: Hugh Dickins <hughd@google.com> but don't bother: I'm sure Andrew will simply fold this fix into the fixed patch later on. Thanks, Hugh > > Best Regards, > Huang, Ying > > -------------------------------------------------------------------------> > From 4b96423796ab7435104eb2cb4dcf5d525b9e0800 Mon Sep 17 00:00:00 2001 > From: Huang Ying <ying.huang@intel.com> > Date: Fri, 17 Feb 2017 10:31:37 +0800 > Subject: [PATCH] mm, swap: Fix memory leak in __read_swap_cache_async() > > The memory may be leaked in __read_swap_cache_async(). For the cases > as below, > > CPU 0 CPU 1 > ----- ----- > > find_get_page() == NULL > __swp_swapcount() != 0 > new_page = alloc_page_vma() > radix_tree_maybe_preload() > swap in swap slot > swapcache_prepare() == -EEXIST > cond_resched() > reclaim the swap slot > find_get_page() == NULL > __swp_swapcount() == 0 > return NULL <- new_page leaked here !!! > > The memory leak has been confirmed via checking the value of new_page > when returning inside the loop in __read_swap_cache_async(). > > This is fixed via replacing return with break inside of loop in > __read_swap_cache_async(), so that there is opportunity for the > new_page to be checked and freed. > > Reported-by: Hugh Dickins <hughd@google.com> > Cc: Tim Chen <tim.c.chen@linux.intel.com> > Signed-off-by: "Huang, Ying" <ying.huang@intel.com> > --- > mm/swap_state.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 2126e9ba23b2..473b71e052a8 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -333,7 +333,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask, > * else swap_off will be aborted if we return NULL. > */ > if (!__swp_swapcount(entry) && swap_slot_cache_enabled) > - return NULL; > + break; > > /* > * Get a new page to read into from swap. > -- > 2.11.0 > > -- 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:[~2017-02-17 18:42 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-02-16 5:22 swap_cluster_info lockdep splat Minchan Kim 2017-02-16 5:22 ` Minchan Kim 2017-02-16 7:13 ` Huang, Ying 2017-02-16 7:13 ` Huang, Ying 2017-02-16 8:44 ` Huang, Ying 2017-02-16 8:44 ` Huang, Ying 2017-02-16 19:00 ` Hugh Dickins 2017-02-16 19:00 ` Hugh Dickins 2017-02-16 19:34 ` Tim Chen 2017-02-16 19:34 ` Tim Chen 2017-02-17 1:46 ` Hugh Dickins 2017-02-17 2:07 ` Huang, Ying 2017-02-17 2:07 ` Huang, Ying 2017-02-17 2:37 ` Huang, Ying 2017-02-17 2:37 ` Huang, Ying 2017-02-17 7:32 ` Huang, Ying 2017-02-17 7:32 ` Huang, Ying 2017-02-17 18:42 ` Hugh Dickins [this message] 2017-02-17 18:42 ` Hugh Dickins 2017-02-16 23:45 ` Minchan Kim 2017-02-16 23:45 ` Minchan Kim 2017-02-17 0:38 ` Huang, Ying 2017-02-17 0:38 ` Huang, Ying
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=alpine.LSU.2.11.1702171036010.1638@eggly.anvils \ --to=hughd@google.com \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=minchan@kernel.org \ --cc=tim.c.chen@linux.intel.com \ --cc=ying.huang@intel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.