Re: + mm-vmscan-add-mm_vmscan_inactive_list_is_low-tracepoint.patch added to -mm tree

[Michal Hocko]

On Wed 11-01-17 08:52:50, Minchan Kim wrote:
[...]
> > @@ -2055,8 +2055,8 @@ static bool inactive_list_is_low(struct
> >  	if (!file && !total_swap_pages)
> >  		return false;
> >  
> > -	inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
> > -	active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
> > +	total_inactive = inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
> > +	total_active = active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
> >  
> 
> the decision of deactivating is based on eligible zone's LRU size,
> not whole zone so why should we need to get a trace of all zones's LRU?

Strictly speaking, the total_ counters are not necessary for making the
decision. I found reporting those numbers useful regardless because this
will give us also an information how large is the eligible portion of
the LRU list. We do not have any other tracepoint which would report
that.
 
[...]
> > @@ -2223,7 +2228,7 @@ static void get_scan_count(struct lruvec
> >  	 * lruvec even if it has plenty of old anonymous pages unless the
> >  	 * system is under heavy pressure.
> >  	 */
> > -	if (!inactive_list_is_low(lruvec, true, sc) &&
> > +	if (!inactive_list_is_low(lruvec, true, sc, false) &&
> 
> Hmm, I was curious why you added trace boolean arguement and found it here.
> Yes, here is not related to deactivation directly but couldn't we help to
> trace it unconditionally?

I've had it like that when I was debugging the mentioned bug and found
it a bit disturbing. It generated more output than I would like and it
wasn't really clear from which code path  this has been called from.

> With that, we can know why VM reclaim only
> file-backed page on slow device although enough anonymous pages on fast
> swap like zram are enough.

That would be something for a separate tracepoint in g_s_c

Thanks!
-- 
Michal Hocko
SUSE Labs