linux-kernel - Re: [PATCH -next] mm/vmscan: fix an undefined behavior for zone id

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] [day] [month] [year] [list]

Message-ID: <20191112183032.GC512@dhcp22.suse.cz>
Date:   Tue, 12 Nov 2019 19:30:32 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     Chris Down <chris@...isdown.name>, Qian Cai <cai@....pw>,
        akpm@...ux-foundation.org, guro@...com, linux-mm@...ck.org,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next] mm/vmscan: fix an undefined behavior for zone id

On Tue 12-11-19 13:20:17, Johannes Weiner wrote:
> On Tue, Nov 12, 2019 at 05:31:56PM +0100, Michal Hocko wrote:
> > On Tue 12-11-19 11:16:58, Johannes Weiner wrote:
> > > On Tue, Nov 12, 2019 at 04:27:50PM +0100, Michal Hocko wrote:
> > > > lruvec_lru_size is explicitly documented to use MAX_NR_ZONES for all
> > > > LRUs and git grep says there are more instances outside of
> > > > get_scan_count. So all of them have to be fixed.
> > > 
> > > Which ones?
> > > 
> > > [hannes@...puter linux]$ git grep lruvec_lru_size
> > > include/linux/mmzone.h:extern unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx);
> > > mm/vmscan.c: * lruvec_lru_size -  Returns the number of pages on the given LRU list.
> > > mm/vmscan.c:unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx)
> > > mm/vmscan.c:    anon  = lruvec_lru_size(lruvec, LRU_ACTIVE_ANON, MAX_NR_ZONES - 1) +
> > > mm/vmscan.c:            lruvec_lru_size(lruvec, LRU_INACTIVE_ANON, MAX_NR_ZONES - 1);
> > > mm/vmscan.c:    file  = lruvec_lru_size(lruvec, LRU_ACTIVE_FILE, MAX_NR_ZONES - 1) +
> > > mm/vmscan.c:            lruvec_lru_size(lruvec, LRU_INACTIVE_FILE, MAX_NR_ZONES - 1);
> > > mm/vmscan.c:            lruvec_size = lruvec_lru_size(lruvec, lru, sc->reclaim_idx);
> > > [hannes@...puter linux]$
> > 
> > I have checked the Linus tree but now double checked with the current
> > next
> > $ git describe next/master
> > next-20191112
> > $ git grep "lruvec_lru_size.*MAX_NR_ZONES" next/master
> > next/master:mm/vmscan.c:                        lruvec_lru_size(lruvec, inactive_lru, MAX_NR_ZONES), inactive,
> > next/master:mm/vmscan.c:                        lruvec_lru_size(lruvec, active_lru, MAX_NR_ZONES), active,
> > next/master:mm/vmscan.c:        anon  = lruvec_lru_size(lruvec, LRU_ACTIVE_ANON, MAX_NR_ZONES) +
> > next/master:mm/vmscan.c:                lruvec_lru_size(lruvec, LRU_INACTIVE_ANON, MAX_NR_ZONES);
> > next/master:mm/vmscan.c:        file  = lruvec_lru_size(lruvec, LRU_ACTIVE_FILE, MAX_NR_ZONES) +
> > next/master:mm/vmscan.c:                lruvec_lru_size(lruvec, LRU_INACTIVE_FILE, MAX_NR_ZONES);
> > next/master:mm/workingset.c:    active_file = lruvec_lru_size(lruvec, LRU_ACTIVE_FILE, MAX_NR_ZONES);
> > 
> > are there any changes which didn't make it to linux next yet?
> 
> Aaahh, that makes sense. I was looking at the latest mmots which
> has
> 
> - mm: vmscan: detect file thrashing at the reclaim root
> - mm: vmscan: enforce inactive:active ratio at the reclaim root
> 
> Those replace the inactive_is_low and the workingset callsites with
> the recursive lruvec_page_state(). It looks like that isn't in next -
> and while I hope it'll make it into 5.5, it might not. So we need a
> fix that considers the other callsites as well.
> 
> Qian's patches that Andrew already has will be good then, as it
> reduces the churn to those other callsites that are in flux.
> 
> We can clean things up when the dust settles.

Yeah, while I am not really super happy about the code that is more
complex than necessary the clean up can be done on top and we can also
think about how to do it properly (I still haven't given up ;))

-- 
Michal Hocko
SUSE Labs