From: Yang Shi <shy828301@gmail.com>
To: David Hildenbrand <david@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
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>
Subject: Re: [PATCH] mm: thp: don't have to lock page anymore when splitting PMD
Date: Fri, 4 Mar 2022 10:30:35 -0800 [thread overview]
Message-ID: <CAHbLzkriyBy2HqjssurLSnhoyuUzpJRZjMPNx34MTgxeO0dddg@mail.gmail.com> (raw)
In-Reply-To: <CADFyXm6W9CVkO4XPYep-tHg55c8m8NES783kcVYrdjSMbzYoDA@mail.gmail.com>
On Thu, Mar 3, 2022 at 9:06 PM David Hildenbrand <david@redhat.com> wrote:
>
> Hi,
>
> This probably bounces on the list due to html junk from the gmail app.
>
> What happened to
>
> https://lore.kernel.org/linux-mm/20220131162940.210846-10-david@redhat.com/
>
> Included in the very series mentioned below?
>
> Was this silently dropped due to folio conversion collisions? :/
I really didn't notice you already proposed this. Maybe folio
conversion, maybe mlock cleanup, I can't tell. But anyway this patch
needs to get rebased. I will submit v2 to solve the comment, will add
your signed-off-by.
>
> Yang Shi <shy828301@gmail.com> schrieb am Do. 3. März 2022 um 23:20:
>>
>> The commit c444eb564fb1 ("mm: thp: make the THP mapcount atomic against
>> __split_huge_pmd_locked()") locked the page for PMD split to make
>> mapcount stable for reuse_swap_page(), then commit 1c2f67308af4 ("mm:
>> thp: fix MADV_REMOVE deadlock on shmem THP") reduce the scope to
>> anonymous page only.
>>
>> However COW has not used mapcount to determine if the page is shared or
>> not anymore due to the COW fixes [1] from David Hildenbrand and the
>> reuse_swap_page() was removed as well. So PMD split doesn't have to
>> lock the page anymore. This patch basically reverted the above two
>> commits.
>>
>> [1] https://lore.kernel.org/linux-mm/20220131162940.210846-1-david@redhat.com/
>>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Cc: Hugh Dickins <hughd@google.com>
>> Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
>> Signed-off-by: Yang Shi <shy828301@gmail.com>
>> ---
>> mm/huge_memory.c | 44 +++++---------------------------------------
>> 1 file changed, 5 insertions(+), 39 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index b49e1a11df2e..daaa698bd273 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2134,8 +2134,6 @@ void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>> {
>> 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,
>> @@ -2148,48 +2146,16 @@ void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>> * pmd against. Otherwise we can end up replacing wrong folio.
>> */
>> VM_BUG_ON(freeze && !folio);
>> - if (folio) {
>> - VM_WARN_ON_ONCE(!folio_test_locked(folio));
>> - if (folio != page_folio(pmd_page(*pmd)))
>> - goto out;
>> - }
>> + if (folio && folio != page_folio(pmd_page(*pmd)))
>> + goto out;
>>
>> -repeat:
>> - if (pmd_trans_huge(*pmd)) {
>> - if (!folio) {
>> - folio = page_folio(pmd_page(*pmd));
>> - /*
>> - * An anonymous page must be locked, to ensure that a
>> - * concurrent reuse_swap_page() sees stable mapcount;
>> - * but reuse_swap_page() is not used on shmem or file,
>> - * and page lock must not be taken when zap_pmd_range()
>> - * calls __split_huge_pmd() while i_mmap_lock is held.
>> - */
>> - if (folio_test_anon(folio)) {
>> - if (unlikely(!folio_trylock(folio))) {
>> - folio_get(folio);
>> - _pmd = *pmd;
>> - spin_unlock(ptl);
>> - folio_lock(folio);
>> - spin_lock(ptl);
>> - if (unlikely(!pmd_same(*pmd, _pmd))) {
>> - folio_unlock(folio);
>> - folio_put(folio);
>> - folio = NULL;
>> - goto repeat;
>> - }
>> - folio_put(folio);
>> - }
>> - do_unlock_folio = true;
>> - }
>> - }
>> - } else if (!(pmd_devmap(*pmd) || is_pmd_migration_entry(*pmd)))
>> + if (!(pmd_devmap(*pmd) || is_pmd_migration_entry(*pmd)))
>> goto out;
>> +
>> __split_huge_pmd_locked(vma, pmd, range.start, freeze);
>> out:
>> spin_unlock(ptl);
>> - if (do_unlock_folio)
>> - folio_unlock(folio);
>> +
>> /*
>> * No need to double call mmu_notifier->invalidate_range() callback.
>> * They are 3 cases to consider inside __split_huge_pmd_locked():
>> --
>> 2.26.3
>>
next prev parent reply other threads:[~2022-03-04 18:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-03 22:20 [PATCH] mm: thp: don't have to lock page anymore when splitting PMD Yang Shi
2022-03-04 2:25 ` Miaohe Lin
2022-03-04 3:12 ` Yang Shi
[not found] ` <CADFyXm6W9CVkO4XPYep-tHg55c8m8NES783kcVYrdjSMbzYoDA@mail.gmail.com>
2022-03-04 18:30 ` Yang Shi [this message]
2022-03-04 18:50 ` David Hildenbrand
2022-03-04 19:01 ` Yang Shi
2022-03-07 2:07 ` Andrew Morton
2022-03-07 8:24 ` David Hildenbrand
2022-03-07 23:43 ` Andrew Morton
2022-03-08 0:03 ` Yang Shi
2022-03-08 0:50 ` Andrew Morton
2022-03-08 0:59 ` Yang Shi
2022-03-08 2:36 ` Hugh Dickins
2022-03-08 3:12 ` Yang Shi
2022-03-08 8:53 ` David Hildenbrand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHbLzkriyBy2HqjssurLSnhoyuUzpJRZjMPNx34MTgxeO0dddg@mail.gmail.com \
--to=shy828301@gmail.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=hughd@google.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).