Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1421025 > unrolled thread

Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Johannes Weiner <hannes@cmpxchg.org> - 2016-06-13 18:00 +0200
    Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Minchan Kim <minchan@kernel.org> - 2016-06-15 04:30 +0200
      Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Johannes Weiner <hannes@cmpxchg.org> - 2016-06-16 17:20 +0200
        Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Minchan Kim <minchan@kernel.org> - 2016-06-17 09:50 +0200
          Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Johannes Weiner <hannes@cmpxchg.org> - 2016-06-17 19:10 +0200
            Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Minchan Kim <minchan@kernel.org> - 2016-06-20 10:20 +0200
              Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing Johannes Weiner <hannes@cmpxchg.org> - 2016-06-23 00:00 +0200

#1421025 — Re: [PATCH 10/10] mm: balance LRU lists based on relative thrashing

On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> On Mon, Jun 06, 2016 at 03:48:36PM -0400, Johannes Weiner wrote:
> > @@ -79,6 +79,7 @@ enum pageflags {
> >  	PG_dirty,
> >  	PG_lru,
> >  	PG_active,
> > +	PG_workingset,
> 
> I think PG_workingset might be a good flag in the future, core MM might
> utilize it to optimize something so I hope it supports for 32bit, too.
> 
> A usecase with PG_workingset in old was cleancache. A few year ago,
> Dan tried it to only cache activated page from page cache to cleancache,
> IIRC. As well, many system using zram(i.e., fast swap) are still 32 bit
> architecture.
> 
> Just an idea. we might be able to move less important flag(i.e., enabled
> in specific configuration, for example, PG_hwpoison or PG_uncached) in 32bit
> to page_extra to avoid allocate extra memory space and charge the bit as
> PG_workingset. :)

Yeah, I do think it should be a core flag. We have the space for it.

> Other concern about PG_workingset is naming. For file-backed pages, it's
> good because file-backed pages started from inactive's head and promoted
> active LRU once two touch so it's likely to be workingset. However,
> for anonymous page, it starts from active list so every anonymous page
> has PG_workingset while mlocked pages cannot have a chance to have it.
> It wouldn't matter in eclaim POV but if we would use PG_workingset as
> indicator to identify real workingset page, it might be confused.
> Maybe, We could mark mlocked pages as workingset unconditionally.

Hm I'm not sure it matters. Technically we don't have to set it on
anon, but since it's otherwise unused anyway, it's nice to set it to
reinforce the notion that anon is currently always workingset.

> > @@ -544,6 +544,8 @@ void migrate_page_copy(struct page *newpage, struct page *page)
> >  		SetPageActive(newpage);
> >  	} else if (TestClearPageUnevictable(page))
> >  		SetPageUnevictable(newpage);
> > +	if (PageWorkingset(page))
> > +		SetPageWorkingset(newpage);
> 
> When I see this, popped thought is how we handle PG_workingset
> when split/collapsing THP and then, I can't find any logic. :(
> Every anonymous page is PG_workingset by birth so you ignore it
> intentionally?

Good catch. __split_huge_page_tail() should copy it over, will fix that.

> > @@ -1809,6 +1811,8 @@ fail_putback:
> >  		mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
> >  
> >  		/* Reverse changes made by migrate_page_copy() */
> > +		if (TestClearPageWorkingset(new_page))
> > +			ClearPageWorkingset(page);
> >  		if (TestClearPageActive(new_page))
> >  			SetPageActive(page);
> >  		if (TestClearPageUnevictable(new_page))
> > diff --git a/mm/swap.c b/mm/swap.c
> > index ae07b469ddca..cb6773e1424e 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -249,8 +249,28 @@ void rotate_reclaimable_page(struct page *page)
> >  	}
> >  }
> >  
> > -void lru_note_cost(struct lruvec *lruvec, bool file, unsigned int nr_pages)
> > +void lru_note_cost(struct lruvec *lruvec, enum lru_cost_type cost,
> > +		   bool file, unsigned int nr_pages)
> >  {
> > +	if (cost == COST_IO) {
> > +		/*
> > +		 * Reflect the relative reclaim cost between incurring
> > +		 * IO from refaults on one hand, and incurring CPU
> > +		 * cost from rotating scanned pages on the other.
> > +		 *
> > +		 * XXX: For now, the relative cost factor for IO is
> > +		 * set statically to outweigh the cost of rotating
> > +		 * referenced pages. This might change with ultra-fast
> > +		 * IO devices, or with secondary memory devices that
> > +		 * allow users continued access of swapped out pages.
> > +		 *
> > +		 * Until then, the value is chosen simply such that we
> > +		 * balance for IO cost first and optimize for CPU only
> > +		 * once the thrashing subsides.
> > +		 */
> > +		nr_pages *= SWAP_CLUSTER_MAX;
> > +	}
> > +
> >  	lruvec->balance.numer[file] += nr_pages;
> >  	lruvec->balance.denom += nr_pages;
> 
> So, lru_cost_type is binary. COST_IO and COST_CPU. 'bool' is enough to
> represent it if you doesn't have further plan to expand it.
> But if you did to make it readable, I'm not against. Just trivial.

Yeah, it's meant for readability. "true" and "false" make for fairly
cryptic arguments when they are a static property of the callsite:

  lru_note_cost(lruvec, false, page_is_file_cache(page), hpage_nr_pages(page))

???

So I'd rather name these things and leave bool for things that are
based on predicate functions.

> > @@ -821,13 +842,28 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
> >  static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
> >  				 void *arg)
> >  {
> > +	unsigned int nr_pages = hpage_nr_pages(page);
> >  	enum lru_list lru = page_lru(page);
> > +	bool active = is_active_lru(lru);
> > +	bool file = is_file_lru(lru);
> > +	bool new = (bool)arg;
> >  
> >  	VM_BUG_ON_PAGE(PageLRU(page), page);
> >  
> >  	SetPageLRU(page);
> >  	add_page_to_lru_list(page, lruvec, lru);
> >  
> > +	if (new) {
> > +		/*
> > +		 * If the workingset is thrashing, note the IO cost of
> > +		 * reclaiming that list and steer reclaim away from it.
> > +		 */
> > +		if (PageWorkingset(page))
> > +			lru_note_cost(lruvec, COST_IO, file, nr_pages);
> > +		else if (active)
> > +			SetPageWorkingset(page);
> > +	}
> > +
> >  	trace_mm_lru_insertion(page, lru);
> >  }
> >  
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 5400f814ae12..43561a56ba5d 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -365,6 +365,7 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
> >  			/*
> >  			 * Initiate read into locked page and return.
> >  			 */
> 
> How about putting the comment you said to Tim in here?
> 
> "
> There are no shadow entries for anonymous evictions, only page cache
> evictions. All swap-ins are treated as "eligible" refaults and push back
> against cache, whereas cache only pushes against anon if the cache
> workingset is determined to fit into memory.
> That implies a fixed hierarchy where the VM always tries to fit the
> anonymous workingset into memory first and the page cache second.
> If the anonymous set is bigger than memory, the algorithm won't stop
> counting IO cost from anonymous refaults and pressuring page cache.
> "
> Or put it in workingset.c. I see you wrote up a little bit about
> anonymous refault in there but I think adding abvove paragraph is
> very helpful.

Agreed, that would probably be helpful. I'll put that in.

Thanks Minchan!

[toc] | [next] | [standalone]

#1422496

On Mon, Jun 13, 2016 at 11:52:31AM -0400, Johannes Weiner wrote:
> On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> > On Mon, Jun 06, 2016 at 03:48:36PM -0400, Johannes Weiner wrote:
> > > @@ -79,6 +79,7 @@ enum pageflags {
> > >  	PG_dirty,
> > >  	PG_lru,
> > >  	PG_active,
> > > +	PG_workingset,
> > 
> > I think PG_workingset might be a good flag in the future, core MM might
> > utilize it to optimize something so I hope it supports for 32bit, too.
> > 
> > A usecase with PG_workingset in old was cleancache. A few year ago,
> > Dan tried it to only cache activated page from page cache to cleancache,
> > IIRC. As well, many system using zram(i.e., fast swap) are still 32 bit
> > architecture.
> > 
> > Just an idea. we might be able to move less important flag(i.e., enabled
> > in specific configuration, for example, PG_hwpoison or PG_uncached) in 32bit
> > to page_extra to avoid allocate extra memory space and charge the bit as
> > PG_workingset. :)
> 
> Yeah, I do think it should be a core flag. We have the space for it.
> 
> > Other concern about PG_workingset is naming. For file-backed pages, it's
> > good because file-backed pages started from inactive's head and promoted
> > active LRU once two touch so it's likely to be workingset. However,
> > for anonymous page, it starts from active list so every anonymous page
> > has PG_workingset while mlocked pages cannot have a chance to have it.
> > It wouldn't matter in eclaim POV but if we would use PG_workingset as
> > indicator to identify real workingset page, it might be confused.
> > Maybe, We could mark mlocked pages as workingset unconditionally.
> 
> Hm I'm not sure it matters. Technically we don't have to set it on
> anon, but since it's otherwise unused anyway, it's nice to set it to
> reinforce the notion that anon is currently always workingset.

When I read your description firstly, I thought the flag for anon page
is set on only swapin but now I feel you want to set it for all of
anonymous page but it has several holes like mlocked pages, shmem pages
and THP and you want to fix it in THP case only.
Hm, What's the rule?
It's not consistent and confusing to me. :(

I think it would be better that PageWorkingset function should return
true in case of PG_swapbacked set if we want to consider all pages of
anonymous LRU PG_workingset which is more clear, not error-prone, IMHO.

Another question:

Do we want to retain [1]?

This patch motivates from swap IO could be much faster than file IO
so that it would be natural if we rely on refaulting feedback rather
than forcing evicting file cache?

[1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?

[toc] | [prev] | [next] | [standalone]

#1424183

On Wed, Jun 15, 2016 at 11:23:41AM +0900, Minchan Kim wrote:
> On Mon, Jun 13, 2016 at 11:52:31AM -0400, Johannes Weiner wrote:
> > On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> > > Other concern about PG_workingset is naming. For file-backed pages, it's
> > > good because file-backed pages started from inactive's head and promoted
> > > active LRU once two touch so it's likely to be workingset. However,
> > > for anonymous page, it starts from active list so every anonymous page
> > > has PG_workingset while mlocked pages cannot have a chance to have it.
> > > It wouldn't matter in eclaim POV but if we would use PG_workingset as
> > > indicator to identify real workingset page, it might be confused.
> > > Maybe, We could mark mlocked pages as workingset unconditionally.
> > 
> > Hm I'm not sure it matters. Technically we don't have to set it on
> > anon, but since it's otherwise unused anyway, it's nice to set it to
> > reinforce the notion that anon is currently always workingset.
> 
> When I read your description firstly, I thought the flag for anon page
> is set on only swapin but now I feel you want to set it for all of
> anonymous page but it has several holes like mlocked pages, shmem pages
> and THP and you want to fix it in THP case only.
> Hm, What's the rule?
> It's not consistent and confusing to me. :(

I think you are might be over thinking this a bit ;)

The current LRU code has a notion of workingset pages, which is anon
pages and multi-referenced file pages. shmem are considered file for
this purpose. That's why anon start out active and files/shmem do
not. This patch adds refaulting pages to the mix.

PG_workingset keeps track of pages that were recently workingset, so
we set it when the page enters the workingset (activations and
refaults, and new anon from the start). The only thing we need out of
this flag is to tell us whether reclaim is going after the workingset
because the LRUs have become too small to hold it.

mlocked pages are not really interesting because not only are they not
evictable, they are entirely exempt from aging. Without aging, we can
not say whether they are workingset or not. We'll just leave the flags
alone, like the active flag right now.

> I think it would be better that PageWorkingset function should return
> true in case of PG_swapbacked set if we want to consider all pages of
> anonymous LRU PG_workingset which is more clear, not error-prone, IMHO.

I'm not sure I see the upside, it would be more branches and code.

> Another question:
> 
> Do we want to retain [1]?
> 
> This patch motivates from swap IO could be much faster than file IO
> so that it would be natural if we rely on refaulting feedback rather
> than forcing evicting file cache?
> 
> [1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?

Yes! We don't want to go after the workingset, whether it be cache or
anonymous, while there is single-use page cache lying around that we
can reclaim for free, with no IO and little risk of future IO. Anon
memory doesn't have this equivalent. Only cache is lazy-reclaimed.

Once the cache refaults, we activate it to reflect the fact that it's
workingset. Only when we run out of single-use cache do we want to
reclaim multi-use pages, and *then* we balance workingsets based on
cost of refetching each side from secondary storage.

[toc] | [prev] | [next] | [standalone]

#1424763

On Thu, Jun 16, 2016 at 11:12:07AM -0400, Johannes Weiner wrote:
> On Wed, Jun 15, 2016 at 11:23:41AM +0900, Minchan Kim wrote:
> > On Mon, Jun 13, 2016 at 11:52:31AM -0400, Johannes Weiner wrote:
> > > On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> > > > Other concern about PG_workingset is naming. For file-backed pages, it's
> > > > good because file-backed pages started from inactive's head and promoted
> > > > active LRU once two touch so it's likely to be workingset. However,
> > > > for anonymous page, it starts from active list so every anonymous page
> > > > has PG_workingset while mlocked pages cannot have a chance to have it.
> > > > It wouldn't matter in eclaim POV but if we would use PG_workingset as
> > > > indicator to identify real workingset page, it might be confused.
> > > > Maybe, We could mark mlocked pages as workingset unconditionally.
> > > 
> > > Hm I'm not sure it matters. Technically we don't have to set it on
> > > anon, but since it's otherwise unused anyway, it's nice to set it to
> > > reinforce the notion that anon is currently always workingset.
> > 
> > When I read your description firstly, I thought the flag for anon page
> > is set on only swapin but now I feel you want to set it for all of
> > anonymous page but it has several holes like mlocked pages, shmem pages
> > and THP and you want to fix it in THP case only.
> > Hm, What's the rule?
> > It's not consistent and confusing to me. :(
> 
> I think you are might be over thinking this a bit ;)
> 
> The current LRU code has a notion of workingset pages, which is anon
> pages and multi-referenced file pages. shmem are considered file for
> this purpose. That's why anon start out active and files/shmem do
> not. This patch adds refaulting pages to the mix.
> 
> PG_workingset keeps track of pages that were recently workingset, so
> we set it when the page enters the workingset (activations and
> refaults, and new anon from the start). The only thing we need out of
> this flag is to tell us whether reclaim is going after the workingset
> because the LRUs have become too small to hold it.

Understood.

Divergence comes from here. It seems you design the page flag for only
aging/balancing logic working well while I am thinking to leverage the
flag to identify real workingset. I mean a anonymous page would be a cold
if it has just cold data for the application which would be swapped
out after a short time and never swap-in until process exits. However,
we put it from active list so that it has PG_workingset but it's cold
page.

Yes, we cannot use the flag for such purpose in this SEQ replacement so
I will not insist on it.

> 
> mlocked pages are not really interesting because not only are they not
> evictable, they are entirely exempt from aging. Without aging, we can
> not say whether they are workingset or not. We'll just leave the flags
> alone, like the active flag right now.
> 
> > I think it would be better that PageWorkingset function should return
> > true in case of PG_swapbacked set if we want to consider all pages of
> > anonymous LRU PG_workingset which is more clear, not error-prone, IMHO.
> 
> I'm not sure I see the upside, it would be more branches and code.
> 
> > Another question:
> > 
> > Do we want to retain [1]?
> > 
> > This patch motivates from swap IO could be much faster than file IO
> > so that it would be natural if we rely on refaulting feedback rather
> > than forcing evicting file cache?
> > 
> > [1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?
> 
> Yes! We don't want to go after the workingset, whether it be cache or
> anonymous, while there is single-use page cache lying around that we
> can reclaim for free, with no IO and little risk of future IO. Anon
> memory doesn't have this equivalent. Only cache is lazy-reclaimed.
> 
> Once the cache refaults, we activate it to reflect the fact that it's
> workingset. Only when we run out of single-use cache do we want to
> reclaim multi-use pages, and *then* we balance workingsets based on
> cost of refetching each side from secondary storage.

If pages in inactive file LRU are really single-use page cache, I agree.

However, how does the logic can work like that?
If reclaimed file pages were part of workingset(i.e., refault happens),
we give the pressure to anonymous LRU but get_scan_count still force to
reclaim file lru until inactive file LRU list size is enough low.

With that, too many file workingset could be evicted although anon swap
is cheaper on fast swap storage?

IOW, refault mechanisme works once inactive file LRU list size is enough
small but small inactive file LRU doesn't guarantee it has only multiple
-use pages. Hm, Isn't it a problem?

[toc] | [prev] | [next] | [standalone]

#1425327

On Fri, Jun 17, 2016 at 04:49:45PM +0900, Minchan Kim wrote:
> On Thu, Jun 16, 2016 at 11:12:07AM -0400, Johannes Weiner wrote:
> > On Wed, Jun 15, 2016 at 11:23:41AM +0900, Minchan Kim wrote:
> > > On Mon, Jun 13, 2016 at 11:52:31AM -0400, Johannes Weiner wrote:
> > > > On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> > > > > Other concern about PG_workingset is naming. For file-backed pages, it's
> > > > > good because file-backed pages started from inactive's head and promoted
> > > > > active LRU once two touch so it's likely to be workingset. However,
> > > > > for anonymous page, it starts from active list so every anonymous page
> > > > > has PG_workingset while mlocked pages cannot have a chance to have it.
> > > > > It wouldn't matter in eclaim POV but if we would use PG_workingset as
> > > > > indicator to identify real workingset page, it might be confused.
> > > > > Maybe, We could mark mlocked pages as workingset unconditionally.
> > > > 
> > > > Hm I'm not sure it matters. Technically we don't have to set it on
> > > > anon, but since it's otherwise unused anyway, it's nice to set it to
> > > > reinforce the notion that anon is currently always workingset.
> > > 
> > > When I read your description firstly, I thought the flag for anon page
> > > is set on only swapin but now I feel you want to set it for all of
> > > anonymous page but it has several holes like mlocked pages, shmem pages
> > > and THP and you want to fix it in THP case only.
> > > Hm, What's the rule?
> > > It's not consistent and confusing to me. :(
> > 
> > I think you are might be over thinking this a bit ;)
> > 
> > The current LRU code has a notion of workingset pages, which is anon
> > pages and multi-referenced file pages. shmem are considered file for
> > this purpose. That's why anon start out active and files/shmem do
> > not. This patch adds refaulting pages to the mix.
> > 
> > PG_workingset keeps track of pages that were recently workingset, so
> > we set it when the page enters the workingset (activations and
> > refaults, and new anon from the start). The only thing we need out of
> > this flag is to tell us whether reclaim is going after the workingset
> > because the LRUs have become too small to hold it.
> 
> Understood.
> 
> Divergence comes from here. It seems you design the page flag for only
> aging/balancing logic working well while I am thinking to leverage the
> flag to identify real workingset. I mean a anonymous page would be a cold
> if it has just cold data for the application which would be swapped
> out after a short time and never swap-in until process exits. However,
> we put it from active list so that it has PG_workingset but it's cold
> page.
> 
> Yes, we cannot use the flag for such purpose in this SEQ replacement so
> I will not insist on it.

Well, I'm designing the flag so that it's useful for the case I am
introducing it for :)

I have no problem with changing its semantics later on if you want to
build on top of it, rename it, anything - so far as the LRU balancing
is unaffected of course.

But I don't think it makes sense to provision it for potential future
cases that may or may not materialize.

> > > Do we want to retain [1]?
> > > 
> > > This patch motivates from swap IO could be much faster than file IO
> > > so that it would be natural if we rely on refaulting feedback rather
> > > than forcing evicting file cache?
> > > 
> > > [1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?
> > 
> > Yes! We don't want to go after the workingset, whether it be cache or
> > anonymous, while there is single-use page cache lying around that we
> > can reclaim for free, with no IO and little risk of future IO. Anon
> > memory doesn't have this equivalent. Only cache is lazy-reclaimed.
> > 
> > Once the cache refaults, we activate it to reflect the fact that it's
> > workingset. Only when we run out of single-use cache do we want to
> > reclaim multi-use pages, and *then* we balance workingsets based on
> > cost of refetching each side from secondary storage.
> 
> If pages in inactive file LRU are really single-use page cache, I agree.
> 
> However, how does the logic can work like that?
> If reclaimed file pages were part of workingset(i.e., refault happens),
> we give the pressure to anonymous LRU but get_scan_count still force to
> reclaim file lru until inactive file LRU list size is enough low.
> 
> With that, too many file workingset could be evicted although anon swap
> is cheaper on fast swap storage?
> 
> IOW, refault mechanisme works once inactive file LRU list size is enough
> small but small inactive file LRU doesn't guarantee it has only multiple
> -use pages. Hm, Isn't it a problem?

It's a trade-off between the cost of detecting a new workingset from a
stream of use-once pages, and the cost of use-once pages impose on the
established workingset.

That's a pretty easy choice, if you ask me. I'd rather ask cache pages
to prove they are multi-use than have use-once pages put pressure on
the workingset.

Sure, a spike like you describe is certainly possible, where a good
portion of the inactive file pages will be re-used in the near future,
yet we evict all of them in a burst of memory pressure when we should
have swapped. That's a worst case scenario for the use-once policy in
a workingset transition.

However, that's much better than use-once pages, which cost no
additional IO to reclaim and do not benefit from being cached at all,
causing the workingset to be trashed or swapped out.

In your scenario, the real multi-use pages will quickly refault and
get activated and the algorithm will adapt to the new circumstances.

[toc] | [prev] | [next] | [standalone]

#1426320

On Fri, Jun 17, 2016 at 01:01:29PM -0400, Johannes Weiner wrote:
> On Fri, Jun 17, 2016 at 04:49:45PM +0900, Minchan Kim wrote:
> > On Thu, Jun 16, 2016 at 11:12:07AM -0400, Johannes Weiner wrote:
> > > On Wed, Jun 15, 2016 at 11:23:41AM +0900, Minchan Kim wrote:
> > > > On Mon, Jun 13, 2016 at 11:52:31AM -0400, Johannes Weiner wrote:
> > > > > On Fri, Jun 10, 2016 at 11:19:35AM +0900, Minchan Kim wrote:
> > > > > > Other concern about PG_workingset is naming. For file-backed pages, it's
> > > > > > good because file-backed pages started from inactive's head and promoted
> > > > > > active LRU once two touch so it's likely to be workingset. However,
> > > > > > for anonymous page, it starts from active list so every anonymous page
> > > > > > has PG_workingset while mlocked pages cannot have a chance to have it.
> > > > > > It wouldn't matter in eclaim POV but if we would use PG_workingset as
> > > > > > indicator to identify real workingset page, it might be confused.
> > > > > > Maybe, We could mark mlocked pages as workingset unconditionally.
> > > > > 
> > > > > Hm I'm not sure it matters. Technically we don't have to set it on
> > > > > anon, but since it's otherwise unused anyway, it's nice to set it to
> > > > > reinforce the notion that anon is currently always workingset.
> > > > 
> > > > When I read your description firstly, I thought the flag for anon page
> > > > is set on only swapin but now I feel you want to set it for all of
> > > > anonymous page but it has several holes like mlocked pages, shmem pages
> > > > and THP and you want to fix it in THP case only.
> > > > Hm, What's the rule?
> > > > It's not consistent and confusing to me. :(
> > > 
> > > I think you are might be over thinking this a bit ;)
> > > 
> > > The current LRU code has a notion of workingset pages, which is anon
> > > pages and multi-referenced file pages. shmem are considered file for
> > > this purpose. That's why anon start out active and files/shmem do
> > > not. This patch adds refaulting pages to the mix.
> > > 
> > > PG_workingset keeps track of pages that were recently workingset, so
> > > we set it when the page enters the workingset (activations and
> > > refaults, and new anon from the start). The only thing we need out of
> > > this flag is to tell us whether reclaim is going after the workingset
> > > because the LRUs have become too small to hold it.
> > 
> > Understood.
> > 
> > Divergence comes from here. It seems you design the page flag for only
> > aging/balancing logic working well while I am thinking to leverage the
> > flag to identify real workingset. I mean a anonymous page would be a cold
> > if it has just cold data for the application which would be swapped
> > out after a short time and never swap-in until process exits. However,
> > we put it from active list so that it has PG_workingset but it's cold
> > page.
> > 
> > Yes, we cannot use the flag for such purpose in this SEQ replacement so
> > I will not insist on it.
> 
> Well, I'm designing the flag so that it's useful for the case I am
> introducing it for :)
> 
> I have no problem with changing its semantics later on if you want to
> build on top of it, rename it, anything - so far as the LRU balancing
> is unaffected of course.
> 
> But I don't think it makes sense to provision it for potential future
> cases that may or may not materialize.

I admit I was so far from the topic. Sorry, Johannes. :)

The reason I guess is naming of the flag. When you introduced the flag,
I popped a vague idea to utilize the flag in future if it represents real
workingset but as I reviewed code, I realized it's not what I want but just
thing to detect activated page before reclaiming. So to me, it looks like
PG_activated rather than PG_workingset. ;-)

> 
> > > > Do we want to retain [1]?
> > > > 
> > > > This patch motivates from swap IO could be much faster than file IO
> > > > so that it would be natural if we rely on refaulting feedback rather
> > > > than forcing evicting file cache?
> > > > 
> > > > [1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?
> > > 
> > > Yes! We don't want to go after the workingset, whether it be cache or
> > > anonymous, while there is single-use page cache lying around that we
> > > can reclaim for free, with no IO and little risk of future IO. Anon
> > > memory doesn't have this equivalent. Only cache is lazy-reclaimed.
> > > 
> > > Once the cache refaults, we activate it to reflect the fact that it's
> > > workingset. Only when we run out of single-use cache do we want to
> > > reclaim multi-use pages, and *then* we balance workingsets based on
> > > cost of refetching each side from secondary storage.
> > 
> > If pages in inactive file LRU are really single-use page cache, I agree.
> > 
> > However, how does the logic can work like that?
> > If reclaimed file pages were part of workingset(i.e., refault happens),
> > we give the pressure to anonymous LRU but get_scan_count still force to
> > reclaim file lru until inactive file LRU list size is enough low.
> > 
> > With that, too many file workingset could be evicted although anon swap
> > is cheaper on fast swap storage?
> > 
> > IOW, refault mechanisme works once inactive file LRU list size is enough
> > small but small inactive file LRU doesn't guarantee it has only multiple
> > -use pages. Hm, Isn't it a problem?
> 
> It's a trade-off between the cost of detecting a new workingset from a
> stream of use-once pages, and the cost of use-once pages impose on the
> established workingset.
> 
> That's a pretty easy choice, if you ask me. I'd rather ask cache pages
> to prove they are multi-use than have use-once pages put pressure on
> the workingset.

Make sense.

> 
> Sure, a spike like you describe is certainly possible, where a good
> portion of the inactive file pages will be re-used in the near future,
> yet we evict all of them in a burst of memory pressure when we should
> have swapped. That's a worst case scenario for the use-once policy in
> a workingset transition.

So, the point is how such case it happens frequently. A scenario I can
think of is that if we use one-cgroup-per-app, many file pages would be
inactive LRU while active LRU is almost empty until reclaim kicks in.
Because normally, parallel reclaim work during launching new app makes
app's startup time really slow. That's why mobile platform uses notifiers
to get free memory in advance via kiling/reclaiming. Anyway, once we get
amount of free memory and lauching new app in a new cgroup, pages would
live his born LRU list(ie, anon: active file: inactive) without aging.

Then, activity manager can set memory.high of less important app-cgroup
to reclaim it with high value swappiness because swap device is much
faster on that system and much bigger anonymous pages compared to file-
backed pages. Surely, activity manager will expect lots of anonymous
pages be able to swap out but unlike expectation, he will see such spike
easily with reclaiming file-backed pages a lot and refault until inactive
file LRU is enough small.

I think it's enough possible scenario in small system one-cgroup-per-
app.

> 
> However, that's much better than use-once pages, which cost no
> additional IO to reclaim and do not benefit from being cached at all,
> causing the workingset to be trashed or swapped out.

I agree removing e9868505987a entirely is dangerous but I think
we need something to prevent such spike. Checking sc->priority might
be helpful. Anyway, I think it's worth to discuss.

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bbfae9a92819..5d5e8e634a06 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2043,6 +2043,7 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
 	 * system is under heavy pressure.
 	 */
 	if (!inactive_list_is_low(lruvec, true) &&
+	    sc->priority >= DEF_PRIORITY - 2 &&
 	    lruvec_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority) {
 		scan_balance = SCAN_FILE;
 		goto out;

> 
> In your scenario, the real multi-use pages will quickly refault and
> get activated and the algorithm will adapt to the new circumstances.

[toc] | [prev] | [next] | [standalone]

#1429129

On Mon, Jun 20, 2016 at 04:42:08PM +0900, Minchan Kim wrote:
> On Fri, Jun 17, 2016 at 01:01:29PM -0400, Johannes Weiner wrote:
> > On Fri, Jun 17, 2016 at 04:49:45PM +0900, Minchan Kim wrote:
> > > On Thu, Jun 16, 2016 at 11:12:07AM -0400, Johannes Weiner wrote:
> > > > On Wed, Jun 15, 2016 at 11:23:41AM +0900, Minchan Kim wrote:
> > > > > Do we want to retain [1]?
> > > > > 
> > > > > This patch motivates from swap IO could be much faster than file IO
> > > > > so that it would be natural if we rely on refaulting feedback rather
> > > > > than forcing evicting file cache?
> > > > > 
> > > > > [1] e9868505987a, mm,vmscan: only evict file pages when we have plenty?
> > > > 
> > > > Yes! We don't want to go after the workingset, whether it be cache or
> > > > anonymous, while there is single-use page cache lying around that we
> > > > can reclaim for free, with no IO and little risk of future IO. Anon
> > > > memory doesn't have this equivalent. Only cache is lazy-reclaimed.
> > > > 
> > > > Once the cache refaults, we activate it to reflect the fact that it's
> > > > workingset. Only when we run out of single-use cache do we want to
> > > > reclaim multi-use pages, and *then* we balance workingsets based on
> > > > cost of refetching each side from secondary storage.
> > > 
> > > If pages in inactive file LRU are really single-use page cache, I agree.
> > > 
> > > However, how does the logic can work like that?
> > > If reclaimed file pages were part of workingset(i.e., refault happens),
> > > we give the pressure to anonymous LRU but get_scan_count still force to
> > > reclaim file lru until inactive file LRU list size is enough low.
> > > 
> > > With that, too many file workingset could be evicted although anon swap
> > > is cheaper on fast swap storage?
> > > 
> > > IOW, refault mechanisme works once inactive file LRU list size is enough
> > > small but small inactive file LRU doesn't guarantee it has only multiple
> > > -use pages. Hm, Isn't it a problem?
> > 
> > It's a trade-off between the cost of detecting a new workingset from a
> > stream of use-once pages, and the cost of use-once pages impose on the
> > established workingset.
> > 
> > That's a pretty easy choice, if you ask me. I'd rather ask cache pages
> > to prove they are multi-use than have use-once pages put pressure on
> > the workingset.
> 
> Make sense.
> 
> > 
> > Sure, a spike like you describe is certainly possible, where a good
> > portion of the inactive file pages will be re-used in the near future,
> > yet we evict all of them in a burst of memory pressure when we should
> > have swapped. That's a worst case scenario for the use-once policy in
> > a workingset transition.
> 
> So, the point is how such case it happens frequently. A scenario I can
> think of is that if we use one-cgroup-per-app, many file pages would be
> inactive LRU while active LRU is almost empty until reclaim kicks in.
> Because normally, parallel reclaim work during launching new app makes
> app's startup time really slow. That's why mobile platform uses notifiers
> to get free memory in advance via kiling/reclaiming. Anyway, once we get
> amount of free memory and lauching new app in a new cgroup, pages would
> live his born LRU list(ie, anon: active file: inactive) without aging.
> 
> Then, activity manager can set memory.high of less important app-cgroup
> to reclaim it with high value swappiness because swap device is much
> faster on that system and much bigger anonymous pages compared to file-
> backed pages. Surely, activity manager will expect lots of anonymous
> pages be able to swap out but unlike expectation, he will see such spike
> easily with reclaiming file-backed pages a lot and refault until inactive
> file LRU is enough small.
> 
> I think it's enough possible scenario in small system one-cgroup-per-
> app.

That's the workingset transition I was talking about. The algorithm is
designed to settle towards stable memory patterns. We can't possibly
remove one of the key components of this - the use-once policy - to
speed up a few seconds of workingset transition when it comes at the
risk of potentially thrashing the workingset for *hours*.

The fact that swap IO can be faster than filesystem IO doesn't change
this at all. The point is that the reclaim and refetch IO cost of
use-once cache is ZERO. Causing swap IO to make room for more and more
unused cache pages doesn't make any sense, no matter the swap speed.

I really don't see the relevance of this discussion to this patch set.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web