| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1379444251-8ulan52g-mutt-n-horiguchi@ah.jp.nec.com>
Date: Tue, 17 Sep 2013 14:57:31 -0400
From: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
Alex Thorlton <athorlton@....com>,
Mel Gorman <mgorman@...e.de>, Andi Kleen <andi@...stfloor.org>,
Michal Hocko <mhocko@...e.cz>,
KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Rik van Riel <riel@...hat.com>,
Andrea Arcangeli <aarcange@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] hugetlbfs: support split page table lock
On Tue, Sep 17, 2013 at 06:18:51PM +0300, Kirill A. Shutemov wrote:
> Naoya Horiguchi wrote:
> > On Mon, Sep 16, 2013 at 01:42:05PM +0300, Kirill A. Shutemov wrote:
> > > Naoya Horiguchi wrote:
> > > > Hi,
> > > >
> > > > Kirill posted split_ptl patchset for thp today, so in this version
> > > > I post only hugetlbfs part. I added Kconfig variables in following
> > > > Kirill's patches (although without CONFIG_SPLIT_*_PTLOCK_CPUS.)
> > > >
> > > > This patch changes many lines, but all are in hugetlbfs specific code,
> > > > so I think we can apply this independent of thp patches.
> > > > -----
> > > > From: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> > > > Date: Fri, 13 Sep 2013 18:12:30 -0400
> > > > Subject: [PATCH v4] hugetlbfs: support split page table lock
> > > >
> > > > Currently all of page table handling by hugetlbfs code are done under
> > > > mm->page_table_lock. So when a process have many threads and they heavily
> > > > access to the memory, lock contention happens and impacts the performance.
> > > >
> > > > This patch makes hugepage support split page table lock so that we use
> > > > page->ptl of the leaf node of page table tree which is pte for normal pages
> > > > but can be pmd and/or pud for hugepages of some architectures.
> > > >
> > > > ChangeLog v4:
> > > > - introduce arch dependent macro ARCH_ENABLE_SPLIT_HUGETLB_PTLOCK
> > > > (only defined for x86 for now)
> > > > - rename USE_SPLIT_PTLOCKS_HUGETLB to USE_SPLIT_HUGETLB_PTLOCKS
> > > >
> > > > ChangeLog v3:
> > > > - disable split ptl for ppc with USE_SPLIT_PTLOCKS_HUGETLB.
> > > > - remove replacement in some architecture dependent code. This is justified
> > > > because an allocation of pgd/pud/pmd/pte entry can race with other
> > > > allocation, not with read/write access, so we can use different locks.
> > > > http://thread.gmane.org/gmane.linux.kernel.mm/106292/focus=106458
> > > >
> > > > ChangeLog v2:
> > > > - add split ptl on other archs missed in v1
> > > > - drop changes on arch/{powerpc,tile}/mm/hugetlbpage.c
> > > >
> > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> > > > ---
> > > > arch/x86/Kconfig | 4 +++
> > > > include/linux/hugetlb.h | 20 +++++++++++
> > > > include/linux/mm_types.h | 2 ++
> > > > mm/Kconfig | 3 ++
> > > > mm/hugetlb.c | 92 +++++++++++++++++++++++++++++-------------------
> > > > mm/mempolicy.c | 5 +--
> > > > mm/migrate.c | 4 +--
> > > > mm/rmap.c | 2 +-
> > > > 8 files changed, 91 insertions(+), 41 deletions(-)
> > > >
> > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > > index 6a5cf6a..5b83d14 100644
> > > > --- a/arch/x86/Kconfig
> > > > +++ b/arch/x86/Kconfig
> > > > @@ -1884,6 +1884,10 @@ config ARCH_ENABLE_SPLIT_PMD_PTLOCK
> > > > def_bool y
> > > > depends on X86_64 || X86_PAE
> > > >
> > > > +config ARCH_ENABLE_SPLIT_HUGETLB_PTLOCK
> > > > + def_bool y
> > > > + depends on X86_64 || X86_PAE
> > > > +
> > > > menu "Power management and ACPI options"
> > > >
> > > > config ARCH_HIBERNATION_HEADER
> > > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > > > index 0393270..2cdac68 100644
> > > > --- a/include/linux/hugetlb.h
> > > > +++ b/include/linux/hugetlb.h
> > > > @@ -80,6 +80,24 @@ extern const unsigned long hugetlb_zero, hugetlb_infinity;
> > > > extern int sysctl_hugetlb_shm_group;
> > > > extern struct list_head huge_boot_pages;
> > > >
> > > > +#if USE_SPLIT_HUGETLB_PTLOCKS
> > > > +#define huge_pte_lockptr(mm, ptep) ({__pte_lockptr(virt_to_page(ptep)); })
> > > > +#else /* !USE_SPLIT_HUGETLB_PTLOCKS */
> > > > +#define huge_pte_lockptr(mm, ptep) ({&(mm)->page_table_lock; })
> > > > +#endif /* USE_SPLIT_HUGETLB_PTLOCKS */
> > > > +
> > > > +#define huge_pte_offset_lock(mm, address, ptlp) \
> > > > +({ \
> > > > + pte_t *__pte = huge_pte_offset(mm, address); \
> > > > + spinlock_t *__ptl = NULL; \
> > > > + if (__pte) { \
> > > > + __ptl = huge_pte_lockptr(mm, __pte); \
> > > > + *(ptlp) = __ptl; \
> > > > + spin_lock(__ptl); \
> > > > + } \
> > > > + __pte; \
> > > > +})
> > > > +
> > >
> > > [ Disclaimer: I don't know much about hugetlb. ]
> > >
> > > I don't think it's correct. Few points:
> > >
> > > - Hugetlb supports multiple page sizes: on x86_64 2M (PMD) and 1G (PUD).
> > > My patchset only implements it for PMD. We don't even initialize
> > > spinlock in struct page for PUD.
> >
> > In hugetlbfs code, we use huge_pte_offset() to get leaf level entries
> > which can be pud or pmd in x86. huge_pte_lockptr() uses this function,
> > so we can always get the correct ptl regardless of hugepage sizes.
> > As for spinlock initialization, you're right. I'll add it on huge_pte_alloc().
>
> Please, don't.
> If USE_SPLIT_PMD_PTLOCKS is true, pmd_alloc_one() will do it for you
> already for PMD table.
OK, sorry for my sloppy suggestion.
> For pud it should be done in pud_alloc_one(), not in hugetlb code.
Right.
> We already have too many special cases for hugetlb. Don't contribute to
> the mess.
OK.
> > > - If we enable split PMD lock we should use it *globally*. With you patch
> > > we can end up with different locks used by hugetlb and rest of kernel
> > > to protect the same PMD table if USE_SPLIT_HUGETLB_PTLOCKS !=
> > > USE_SPLIT_PMD_PTLOCKS. It's just broken.
> >
> > I don't think so. Thp specific operations (like thp allocation, split,
> > and collapse) are never called on the virtual address range covered by
> > vma(VM_HUGETLB) by checking VM_HUGETLB. So no one tries to lock/unlock
> > a ptl concurrently from thp context and hugetlbfs context.
>
> Two vma's can be next to each other and share the same PMD table (not
> entries) and in this case I don't see what will serialize pmd_alloc() if
> USE_SPLIT_HUGETLB_PTLOCKS != USE_SPLIT_PMD_PTLOCKS.
OK, we can't split thp code and hugetlbfs code finally.
USE_SPLIT_HUGETLB_PTLOCKS must depend on USE_SPLIT_PMD_PTLOCKS.
Thanks,
Naoya
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists