On Fri, Sep 04, 2020 at 12:50:01PM +0200, Michal Hocko wrote:
> 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.
Please bear with me. This is the change in question:
if (put_page_testzero(page)) {
__ClearPageLRU(page);
- __ClearPageActive(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec,
page_off_lru(page));
if (unlikely(PageCompound(page))) {
If the PageUnevictable() check in page_off_lru() is not a concern,
I'm trying to understand what else is different between them:
Before this path: After this patch:
page_off_lru()
__ClearPageActive() __ClearPageActive()
add_page_to_lru_list() add_page_to_lru_list()
And why is page_off_lru() hidden in a release path?
