linux-kernel - Re: + mm-vmscan-add-mm_vmscan_inactive_list_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Jan 2017 16:52:39 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Minchan Kim <minchan@...nel.org>
Cc:     linux-kernel@...r.kernel.org, hillf.zj@...baba-inc.com,
        mgorman@...e.de, vbabka@...e.cz, mm-commits@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: + mm-vmscan-add-mm_vmscan_inactive_list_is_low-tracepoint.patch
 added to -mm tree

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