Re: [rfc patch 3/3] mm: munlock COW pages on truncation unmap

[KOSAKI Motohiro]

> ---
> From: Johannes Weiner <hannes@xxxxxxxxxxx>
> Subject: mm: order evictable rescue in LRU putback
> 
> Isolators putting a page back to the LRU do not hold the page lock,
> and if the page is mlocked, another thread might munlock it
> concurrently.
> 
> Expecting this, the putback code re-checks the evictability of a page
> when it just moved it to the unevictable list in order to correct its
> decision.
> 
> The problem, however, is that ordering is not garuanteed between
> setting PG_lru when moving the page to the list and checking
> PG_mlocked afterwards:
> 
> 	#0 putback			#1 munlock
> 
> 	spin_lock()
> 					if (TestClearPageMlocked())
> 					  if (PageLRU())
> 					    move to evictable list
> 	SetPageLRU()
> 	spin_unlock()
> 	if (!PageMlocked())
> 	  move to evictable list
> 
> The PageMlocked() reading may get reordered before SetPageLRU() in #0,
> resulting in #0 not moving the still mlocked page, and in #1 failing
> to isolate and move the page as well.  The evictable page is now
> stranded on the unevictable list.
> 
> TestClearPageMlocked() in #1 already provides full memory barrier
> semantics.
> 
> This patch adds an explicit full barrier to force ordering between
> SetPageLRU() and PageMlocked() in #0 so that either one of the
> competitors rescues the page.
> 
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> Cc: Hugh Dickins <hugh.dickins@xxxxxxxxxxxxx>
> Cc: Mel Gorman <mel@xxxxxxxxx>
> Cc: Lee Schermerhorn <Lee.Schermerhorn@xxxxxx>
> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> ---
>  mm/vmscan.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -544,6 +544,16 @@ redo:
>  		 */
>  		lru = LRU_UNEVICTABLE;
>  		add_page_to_unevictable_list(page);
> +		/*
> +		 * When racing with an mlock clearing (page is
> +		 * unlocked), make sure that if the other thread does
> +		 * not observe our setting of PG_lru and fails
> +		 * isolation, we see PG_mlocked cleared below and move
> +		 * the page back to the evictable list.
> +		 *
> +		 * The other side is TestClearPageMlocked().
> +		 */
> +		smp_mb();
>  	}

IA64 is most relax cpu reorder architecture. I'm usually test on it
and my test found no problem.
Then, I don't think this issue occur in the real world. but I think
this patch is right.

Hannes, you are great.

	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/