Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1579619

Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in zap_pmd_range()

Show all headers | View raw

On Sun, Feb 12, 2017 at 06:25:09PM -0600, Zi Yan wrote:
> Hi Kirill,
> 
> >>>> The crash scenario I guess is like:
> >>>> 1. A huge page pmd entry is in the middle of being changed into either a
> >>>> pmd_protnone or a pmd_migration_entry. It is cleared to pmd_none.
> >>>>
> >>>> 2. At the same time, the application frees the vma this page belongs to.
> >>>
> >>> Em... no.
> >>>
> >>> This shouldn't be possible: your 1. must be done under down_read(mmap_sem).
> >>> And we only be able to remove vma under down_write(mmap_sem), so the
> >>> scenario should be excluded.
> >>>
> >>> What do I miss?
> >>
> >> You are right. This problem will not happen in the upstream kernel.
> >>
> >> The problem comes from my customized kernel, where I migrate pages away
> >> instead of reclaiming them when memory is under pressure. I did not take
> >> any mmap_sem when I migrate pages. So I got this error.
> >>
> >> It is a false alarm. Sorry about that. Thanks for clarifying the problem.
> >
> > I think there's still a race between MADV_DONTNEED and
> > change_huge_pmd(.prot_numa=1) resulting in skipping THP by
> > zap_pmd_range(). It need to be addressed.
> >
> > And MADV_FREE requires a fix.
> >
> > So, minus one non-bug, plus two bugs.
> >
> 
> You said a huge page pmd entry needs to be changed under down_read(mmap_sem).
> It is only true for huge pages, right?

mmap_sem is a way to make sure that the VMA will not go away under you.
Besides mmap_sem, anon_vma_lock/i_mmap_lock can be used for this.

> Since in mm/compaction.c, the kernel does not down_read(mmap_sem) during memory
> compaction. Namely, base page migrations do not hold down_read(mmap_sem),
> so in zap_pte_range(), the kernel needs to hold PTE page table locks.
> Am I right about this?
> 
> If yes. IMHO, ultimately, when we need to compact 2MB pages to form 1GB pages,
> in zap_pmd_range(), pmd locks have to be taken to make that kind of compactions
> possible.
> 
> Do you agree?

I *think* we can get away with speculative (without ptl) check in
zap_pmd_range() if we make page fault the only place that can turn
pmd_none() into something else.

It means all other sides that change pmd must not clear it intermittently
during pmd change, unless run under down_write(mmap_sem).

I found two such problematic places in kernel:

 - change_huge_pmd(.prot_numa=1);

 - madvise_free_huge_pmd();

Both clear pmd before setting up a modified version. Both under
down_read(mmap_sem).

The migration path also would need to establish migration pmd atomically
to make this work.

Once all these cases will be fixed, zap_pmd_range() would only be able to
race with page fault if it called from MADV_DONTNEED.
This case is not a problem.

Andrea, does it sound reasonable to you?

-- 
 Kirill A. Shutemov

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 00/14] mm: page migration enhancement for thp Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
  [PATCH v3 11/14] mm: hwpoison: soft offline supports thp migration Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
  [PATCH v3 13/14] mm: migrate: move_pages() supports thp migration Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
    Re: [PATCH v3 13/14] mm: migrate: move_pages() supports thp  migration Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-09 10:30 +0100
      Re: [PATCH v3 13/14] mm: migrate: move_pages() supports thp migration "Zi Yan" <zi.yan@sent.com> - 2017-02-09 18:40 +0100
  [PATCH v3 04/14] mm: x86: move _PAGE_SWP_SOFT_DIRTY from bit 7 to bit 1 Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
    Re: [PATCH v3 04/14] mm: x86: move _PAGE_SWP_SOFT_DIRTY from bit 7  to bit 1 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-09 10:30 +0100
  [PATCH v3 03/14] mm: use pmd lock instead of racy checks in zap_pmd_range() Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
    Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in zap_pmd_range() "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-02-06 05:10 +0100
    Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-06 08:50 +0100
      Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Zi Yan" <zi.yan@sent.com> - 2017-02-06 14:10 +0100
        Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-07 00:30 +0100
    Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-06 17:10 +0100
      Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Zi Yan" <zi.yan@sent.com> - 2017-02-06 17:40 +0100
        Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-06 18:40 +0100
      Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in zap_pmd_range() "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> - 2017-02-07 15:00 +0100
    Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-07 15:30 +0100
      Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-07 17:40 +0100
        Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-07 19:00 +0100
          Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-02-13 12:00 +0100
            Re: [PATCH v3 03/14] mm: use pmd lock instead of racy checks in  zap_pmd_range() Andrea Arcangeli <aarcange@redhat.com> - 2017-02-13 15:50 +0100
  [PATCH v3 01/14] mm: thp: make __split_huge_pmd_locked visible. Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
    Re: [PATCH v3 01/14] mm: thp: make __split_huge_pmd_locked visible. Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-06 07:30 +0100
      Re: [PATCH v3 01/14] mm: thp: make __split_huge_pmd_locked visible. "Zi Yan" <zi.yan@sent.com> - 2017-02-06 13:20 +0100
    Re: [PATCH v3 01/14] mm: thp: make __split_huge_pmd_locked visible. Matthew Wilcox <willy@infradead.org> - 2017-02-06 16:10 +0100
      Re: [PATCH v3 01/14] mm: thp: make __split_huge_pmd_locked visible. "Zi Yan" <zi.yan@sent.com> - 2017-02-06 16:10 +0100
  [PATCH v3 09/14] mm: thp: check pmd migration entry in common path Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
    Re: [PATCH v3 09/14] mm: thp: check pmd migration entry in common  path Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> - 2017-02-09 10:30 +0100
  [PATCH v3 07/14] mm: thp: introduce CONFIG_ARCH_ENABLE_THP_MIGRATION Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
  [PATCH v3 05/14] mm: mempolicy: add queue_pages_node_check() Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
  [PATCH v3 14/14] mm: memory_hotplug: memory hotremove supports thp migration Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100
  [PATCH v3 02/14] mm: thp: create new __zap_huge_pmd_locked function. Zi Yan <zi.yan@sent.com> - 2017-02-05 17:20 +0100

csiph-web