[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Yim+aOCyfwLrqYWi@dhcp22.suse.cz>
Date: Thu, 10 Mar 2022 10:01:28 +0100
From: Michal Hocko <mhocko@...e.com>
To: Yu Zhao <yuzhao@...gle.com>
Cc: Minchan Kim <minchan@...nel.org>,
Ivan Teterevkov <ivan.teterevkov@...anix.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Linux-MM <linux-mm@...ck.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-api@...r.kernel.org, Johannes Weiner <hannes@...xchg.org>,
Tim Murray <timmurray@...gle.com>,
Joel Fernandes <joel@...lfernandes.org>,
Suren Baghdasaryan <surenb@...gle.com>, dancol@...gle.com,
Shakeel Butt <shakeelb@...gle.com>, sonnyrao@...gle.com,
oleksandr@...hat.com, Hillf Danton <hdanton@...a.com>,
lizeb@...gle.com, Dave Hansen <dave.hansen@...el.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: Regression of madvise(MADV_COLD) on shmem?
On Mon 07-03-22 13:10:08, Michal Hocko wrote:
> On Sat 05-03-22 02:17:37, Yu Zhao wrote:
> [...]
> > diff --git a/mm/swap.c b/mm/swap.c
> > index bcf3ac288b56..7fd99f037ca7 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -563,7 +559,7 @@ static void lru_deactivate_file_fn(struct page
> > *page, struct lruvec *lruvec)
> >
> > static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec)
> > {
> > - if (PageActive(page) && !PageUnevictable(page)) {
> > + if (!PageUnevictable(page)) {
> > int nr_pages = thp_nr_pages(page);
> >
> > del_page_from_lru_list(page, lruvec);
> > @@ -677,7 +673,7 @@ void deactivate_file_page(struct page *page)
> > */
> > void deactivate_page(struct page *page)
> > {
> > - if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
> > + if (PageLRU(page) && !PageUnevictable(page)) {
> > struct pagevec *pvec;
> >
> > local_lock(&lru_pvecs.lock);
> >
> > I'll leave it to Minchan to decide whether this is worth fixing,
> > together with this one:
>
> There doesn't seem to be any dependency on the PageActive anymore. I do
> remember we have relied on the PageActive to move from the active list
> to the inactive. This is not the case anymore but I am wondering whether
> above is really sufficient. If you are deactivating an inactive page
> then I would expect you want to move that page in the LRU as well. In
> other words don't you want
> if (page_active)
> add_page_to_lru_list
> else
> add_page_to_lru_list_tail
Do you plan to send an official patch?
--
Michal Hocko
SUSE Labs
Powered by blists - more mailing lists