Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages

From	Minchan Kim <minchan@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages
Date	2017-02-25 00:30 +0100
Message-ID	<textf-7sW-13@gated-at.bofh.it> (permalink)
References	<tdKiR-62X-7@gated-at.bofh.it> <tdKiT-62X-49@gated-at.bofh.it> <tedEd-1M7-5@gated-at.bofh.it> <teqi6-2bp-25@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

Hi Johannes,

On Fri, Feb 24, 2017 at 10:36:55AM -0500, Johannes Weiner wrote:
> On Fri, Feb 24, 2017 at 11:12:18AM +0900, Minchan Kim wrote:
> > > @@ -1525,8 +1531,8 @@ int try_to_unmap(struct page *page, enum ttu_flags flags)
> > >  
> > >  	if (ret != SWAP_MLOCK && !page_mapcount(page)) {
> > >  		ret = SWAP_SUCCESS;
> > > -		if (rp.lazyfreed && !PageDirty(page))
> > > -			ret = SWAP_LZFREE;
> > > +		if (rp.lazyfreed && PageDirty(page))
> > > +			ret = SWAP_DIRTY;
> > 
> > Hmm, I don't understand why we need to introduce new return value.
> > Can't we set SetPageSwapBacked and return SWAP_FAIL in try_to_unmap_one?
> 
> I think that's a bad idea. A function called "try_to_unmap" shouldn't
> have as a side effect that it changes the page's LRU type in an error
> case. Let try_to_unmap be about unmapping the page. If it fails, make
> it report why and let the caller deal with the fallout. Any LRU fixups
> are much better placed in vmscan.c.

I don't think it's page's LRU type change. SetPageSwapBacked is just
indication that page is swappable or not.
Like mlock_vma_page in try_to_unmap_one, we can set SetPageSwapBacked
if we found the lazyfree page dirty. If we don't need to move dirty
lazyfree page to another LRU list, it would be better to not introduce
new return value in try_to_unmap.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH V4 0/6] mm: fix some MADV_FREE issues Shaohua Li <shli@fb.com> - 2017-02-22 20:00 +0100
  [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE list Shaohua Li <shli@fb.com> - 2017-02-22 20:00 +0100
    Re: [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE  list Johannes Weiner <hannes@cmpxchg.org> - 2017-02-23 17:10 +0100
      Re: [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE  list Johannes Weiner <hannes@cmpxchg.org> - 2017-02-23 20:00 +0100
    Re: [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE  list Minchan Kim <minchan@kernel.org> - 2017-02-24 03:00 +0100
      Re: [PATCH V4 3/6] mm: move MADV_FREE pages into LRU_INACTIVE_FILE  list Minchan Kim <minchan@kernel.org> - 2017-02-25 00:40 +0100
  [PATCH V4 5/6] mm: enable MADV_FREE for swapless system Shaohua Li <shli@fb.com> - 2017-02-22 20:00 +0100
  [PATCH V4 2/6] mm: don't assume anonymous pages have SwapBacked flag Shaohua Li <shli@fb.com> - 2017-02-22 20:00 +0100
  [PATCH V4 4/6] mm: reclaim MADV_FREE pages Shaohua Li <shli@fb.com> - 2017-02-22 20:00 +0100
    Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages Johannes Weiner <hannes@cmpxchg.org> - 2017-02-23 17:20 +0100
    Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages Minchan Kim <minchan@kernel.org> - 2017-02-24 03:20 +0100
      Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages Johannes Weiner <hannes@cmpxchg.org> - 2017-02-24 16:50 +0100
        Re: [PATCH V4 4/6] mm: reclaim MADV_FREE pages Minchan Kim <minchan@kernel.org> - 2017-02-25 00:30 +0100

csiph-web