Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220303222014.517033-1-shy828301@gmail.com> <CADFyXm6W9CVkO4XPYep-tHg55c8m8NES783kcVYrdjSMbzYoDA@mail.gmail.com>
 <CAHbLzkriyBy2HqjssurLSnhoyuUzpJRZjMPNx34MTgxeO0dddg@mail.gmail.com>
 <13ad4ba1-2a88-9459-3995-70af36aba33e@redhat.com> <20220306180718.6d4e6233130b94fdad98df88@linux-foundation.org>
 <84a6c9c1-d18f-6955-2666-0a2d7bce6094@redhat.com> <20220307154326.6494f7fca7def491a23e5df1@linux-foundation.org>
 <CAHbLzkqY69nFniK+ckQaeu117Q+OqZ2hvFEdVbZaW2C_zAzDbg@mail.gmail.com> <20220307165015.77fbcbe9e1fa2fe15f3eb239@linux-foundation.org>
In-Reply-To: <20220307165015.77fbcbe9e1fa2fe15f3eb239@linux-foundation.org>
From:   Yang Shi <shy828301@gmail.com>
Date:   Mon, 7 Mar 2022 16:59:27 -0800
Message-ID: <CAHbLzkrHWjDk5PHJPep=KVHmHtktJana9FtAWZoT196Rg9utJA@mail.gmail.com>
Subject: Re: [PATCH] mm: thp: don't have to lock page anymore when splitting PMD
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     David Hildenbrand <david@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Hugh Dickins <hughd@google.com>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux MM <linux-mm@kvack.org>,
        Matthew Wilcox <willy@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Mar 7, 2022 at 4:50 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon, 7 Mar 2022 16:03:12 -0800 Yang Shi <shy828301@gmail.com> wrote:
>
> > On Mon, Mar 7, 2022 at 3:43 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > @@ -2133,8 +2133,6 @@ void __split_huge_pmd(struct vm_area_str
> > >  {
> > >         spinlock_t *ptl;
> > >         struct mmu_notifier_range range;
> > > -       bool do_unlock_folio = false;
> > > -       pmd_t _pmd;
> > >
> > >         mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, vma->vm_mm,
> > >                                 address & HPAGE_PMD_MASK,
> > > @@ -2153,42 +2151,14 @@ void __split_huge_pmd(struct vm_area_str
> > >                         goto out;
> > >         }
> > >
> > > -repeat:
> > >         if (pmd_trans_huge(*pmd)) {
> > > -               if (!folio) {
> > > +               if (!folio)
> > >                         folio = page_folio(pmd_page(*pmd));
> >
> > We could remove the "if (pmd_trans_huge(*pmd))" section since folio is
> > actually not used afterward at all.
>
> >
> > ...
> >
> >
> > With the above if removed, this could be changed to:
> >
> > if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) ||
> >             is_pmd_migration_entry(*pmd))
> >                 __split_huge_pmd_locked(vma, pmd, range.start, freeze);
> >
>
> OK, looks sane.  Can someone please test all this?

Build test? I had the exact same change in my tree, it worked for me.


>
> --- a/mm/huge_memory.c~mm-huge_memory-remove-stale-locking-logic-from-__split_huge_pmd-fix
> +++ a/mm/huge_memory.c
> @@ -2151,12 +2151,10 @@ void __split_huge_pmd(struct vm_area_str
>                         goto out;
>         }
>
> -       if (pmd_trans_huge(*pmd)) {
> -               if (!folio)
> -                       folio = page_folio(pmd_page(*pmd));
> -       } else if (!(pmd_devmap(*pmd) || is_pmd_migration_entry(*pmd)))
> -               goto out;
> -       __split_huge_pmd_locked(vma, pmd, range.start, freeze);
> +       if (pmd_trans_huge(*pmd) || pmd_devmap(*pmd) ||
> +           is_pmd_migration_entry(*pmd)))
> +               __split_huge_pmd_locked(vma, pmd, range.start, freeze);
> +
>  out:
>         spin_unlock(ptl);
>         /*
> _
>