Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210731063938.1391602-1-yuzhao@google.com> <20210731063938.1391602-3-yuzhao@google.com>
 <20210804142257.tsgzpfc3zq7um7iu@box.shutemov.name>
In-Reply-To: <20210804142257.tsgzpfc3zq7um7iu@box.shutemov.name>
From:   Yu Zhao <yuzhao@google.com>
Date:   Sun, 8 Aug 2021 11:28:37 -0600
Message-ID: <CAOUHufZfMTi6_V_hw26WXapMpUuFgT6qnupYkcmkz=qGBWwHsw@mail.gmail.com>
Subject: Re: [PATCH 2/3] mm: free zapped tail pages when splitting isolated thp
To:     "Kirill A. Shutemov" <kirill@shutemov.name>
Cc:     Linux-MM <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Hugh Dickins <hughd@google.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Matthew Wilcox <willy@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Yang Shi <shy828301@gmail.com>, Zi Yan <ziy@nvidia.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Shuang Zhai <zhais@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Aug 4, 2021 at 8:22 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
>
> On Sat, Jul 31, 2021 at 12:39:37AM -0600, Yu Zhao wrote:
> > @@ -2506,6 +2508,25 @@ static void __split_huge_page(struct page *page, struct list_head *list,
> >                       continue;
> >               unlock_page(subpage);
> >
> > +             /*
> > +              * If a tail page has only two references left, one inherited
> > +              * from the isolation of its head and the other from
> > +              * lru_add_page_tail() which we are about to drop, it means this
> > +              * tail page was concurrently zapped. Then we can safely free it
> > +              * and save page reclaim or migration the trouble of trying it.
> > +              */
> > +             if (list && page_ref_freeze(subpage, 2)) {
> > +                     VM_BUG_ON_PAGE(PageLRU(subpage), subpage);
> > +                     VM_BUG_ON_PAGE(PageCompound(subpage), subpage);
> > +                     VM_BUG_ON_PAGE(page_mapped(subpage), subpage);
> > +
> > +                     ClearPageActive(subpage);
> > +                     ClearPageUnevictable(subpage);
>
> Why touch PG_Active/PG_Unevictable?

Subpages may inherit these flags from their isolated head. Page
reclaim doesn't isolate active or unevictable. But migration does. If
we don't clear them here, we'll hit bad_page() later because both
flags are included in PAGE_FLAGS_CHECK_AT_FREE. Does it make sense?
Thanks.