Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Mar 2022 10:01:28 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Yu Zhao <yuzhao@google.com>
Cc:     Minchan Kim <minchan@kernel.org>,
        Ivan Teterevkov <ivan.teterevkov@nutanix.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux-MM <linux-mm@kvack.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-api@vger.kernel.org, Johannes Weiner <hannes@cmpxchg.org>,
        Tim Murray <timmurray@google.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Suren Baghdasaryan <surenb@google.com>, dancol@google.com,
        Shakeel Butt <shakeelb@google.com>, sonnyrao@google.com,
        oleksandr@redhat.com, Hillf Danton <hdanton@sina.com>,
        lizeb@google.com, Dave Hansen <dave.hansen@intel.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: Regression of madvise(MADV_COLD) on shmem?
Message-ID: <Yim+aOCyfwLrqYWi@dhcp22.suse.cz>
References: <dd620dbd-6d71-7553-d1e9-95676ff12c82@nutanix.com>
 <YiKsUr1FQwmDP7V0@google.com>
 <CAOUHufZyrLrkEvFay+FddN5Ve3v_-JvNROHo9hEhsoVuCpwh0Q@mail.gmail.com>
 <YiX2HcF5o2IjfX1H@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YiX2HcF5o2IjfX1H@dhcp22.suse.cz>
Precedence: bulk

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