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:   Fri, 4 Sep 2020 12:50:01 +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>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] mm: use
 add_page_to_lru_list()/page_lru()/page_off_lru()

On Thu 03-09-20 21:24:00, Yu Zhao wrote:
> On Thu, Sep 03, 2020 at 10:28:32AM +0200, Michal Hocko wrote:
> > On Mon 31-08-20 11:50:41, Yu Zhao wrote:
> > [...]
> > > @@ -1860,16 +1859,11 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> > >  		lruvec = mem_cgroup_page_lruvec(page, pgdat);
> > >  
> > >  		SetPageLRU(page);
> > > -		lru = page_lru(page);
> > > -
> > > -		nr_pages = thp_nr_pages(page);
> > > -		update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
> > > -		list_move(&page->lru, &lruvec->lists[lru]);
> > > +		add_page_to_lru_list(page, lruvec, page_lru(page));
> > >  
> > >  		if (put_page_testzero(page)) {
> > >  			__ClearPageLRU(page);
> > > -			__ClearPageActive(page);
> > 
> > This should go into its own patch. The rest is a mechanical and clear.
> 
> Thanks for reviewing.
> 
> I assume you are worrying about PG_unevictable being set on the page
> because page_off_lru() checks it first.

No, I was referring to __ClearPageActive. You are right that this is
cleared in page_off_lru but that is hidden in a release path and e.g.
compound pages are released via their destructor which for some might
not involve releasing the page - e.g. hugetlb pages. This should be fine
because hugetlb pages are not on LRU so as I've said this is fine but it
belongs to its own patch because it is not a pure mechanical change like
the rest of the patch.
-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists