linux-kernel - Re: [PATCH 02/13] mm: use page_off

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 18 Sep 2020 13:09:14 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     Yu Zhao <yuzhao@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Alex Shi <alex.shi@...ux.alibaba.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Roman Gushchin <guro@...com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Chris Down <chris@...isdown.name>,
        Yafang Shao <laoar.shao@...il.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Huang Ying <ying.huang@...el.com>,
        Pankaj Gupta <pankaj.gupta.linux@...il.com>,
        Matthew Wilcox <willy@...radead.org>,
        Konstantin Khlebnikov <koct9i@...il.com>,
        Minchan Kim <minchan@...nel.org>,
        Jaewon Kim <jaewon31.kim@...sung.com>, cgroups@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/13] mm: use page_off_lru()

On Fri 18-09-20 04:27:13, Yu Zhao wrote:
> On Fri, Sep 18, 2020 at 09:37:00AM +0200, Michal Hocko wrote:
> > On Thu 17-09-20 21:00:40, Yu Zhao wrote:
> > > This patch replaces the only open-coded __ClearPageActive() with
> > > page_off_lru(). There is no open-coded __ClearPageUnevictable()s.
> > > 
> > > Before this patch, we have:
> > > 	__ClearPageActive()
> > > 	add_page_to_lru_list()
> > > 
> > > After this patch, we have:
> > > 	page_off_lru()
> > > 		if PageUnevictable()
> > > 			__ClearPageUnevictable()
> > > 		else if PageActive()
> > > 			__ClearPageActive()
> > > 	add_page_to_lru_list()
> > > 
> > > Checking PageUnevictable() shouldn't be a problem because these two
> > > flags are mutually exclusive. Leaking either will trigger bad_page().
> > 
> > I am sorry but the changelog is really hard to grasp. What are you
> > trying to achieve, why and why it is safe. This should be a general
> > outline for any patch. I have already commented on the previous patch
> > and asked you for the explanation why removing __ClearPageActive from
> > this path is desirable and safe. I have specifically asked to clarify
> > the compound page situation as that is using its oen destructor in the
> > freeing path and that might result in page_off_lru to be not called.
> 
> Haven't I explained we are NOT removing __ClearPageActive()? Is my
> notion of the code structure above confusing you? Or 'open-coded'
> could mean different things?

Please read through my reply carefuly. I am not saying what you are
doing is wrong. I am expressing a lack of justification which is the
case throughout this patch series. You do not explain why we need it and
why reviewers should spend time on this. Because the review is not as
trivial as looking at the diff.
-- 
Michal Hocko
SUSE Labs