All of lore.kernel.org
 help / color / mirror / Atom feed
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>

  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: link
Be 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.