lkml.org 
[lkml]   [2022]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 6/6] mm/hugetlb: make detecting shared pte more reliable
    On 08/16/22 21:05, Miaohe Lin wrote:
    > If the pagetables are shared, we shouldn't copy or take references. Since
    > src could have unshared and dst shares with another vma, huge_pte_none()
    > is thus used to determine whether dst_pte is shared. But this check isn't
    > reliable. A shared pte could have pte none in pagetable in fact. The page
    > count of ptep page should be checked here in order to reliably determine
    > whether pte is shared.
    >
    > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
    > ---
    > mm/hugetlb.c | 16 ++++++----------
    > 1 file changed, 6 insertions(+), 10 deletions(-)

    You are correct, this is a better/more reliable way to check for pmd sharing.
    It is accurate since we hold i_mmap_rwsem. I like it.

    Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
    --
    Mike Kravetz

    Note to self, this will not work if we move to vma based locking for pmd
    sharing and do not hold i_mmap_rwsem here.

    >
    > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
    > index e1356ad57087..25db6d07479e 100644
    > --- a/mm/hugetlb.c
    > +++ b/mm/hugetlb.c
    > @@ -4795,15 +4795,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
    >
    > /*
    > * If the pagetables are shared don't copy or take references.
    > - * dst_pte == src_pte is the common case of src/dest sharing.
    > *
    > + * dst_pte == src_pte is the common case of src/dest sharing.
    > * However, src could have 'unshared' and dst shares with
    > - * another vma. If dst_pte !none, this implies sharing.
    > - * Check here before taking page table lock, and once again
    > - * after taking the lock below.
    > + * another vma. So page_count of ptep page is checked instead
    > + * to reliably determine whether pte is shared.
    > */
    > - dst_entry = huge_ptep_get(dst_pte);
    > - if ((dst_pte == src_pte) || !huge_pte_none(dst_entry)) {
    > + if (page_count(virt_to_page(dst_pte)) > 1) {
    > addr |= last_addr_mask;
    > continue;
    > }
    > @@ -4814,11 +4812,9 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
    > entry = huge_ptep_get(src_pte);
    > dst_entry = huge_ptep_get(dst_pte);
    > again:
    > - if (huge_pte_none(entry) || !huge_pte_none(dst_entry)) {
    > + if (huge_pte_none(entry)) {
    > /*
    > - * Skip if src entry none. Also, skip in the
    > - * unlikely case dst entry !none as this implies
    > - * sharing with another vma.
    > + * Skip if src entry none.
    > */
    > ;
    > } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) {
    > --
    > 2.23.0
    >

    \
     
     \ /
      Last update: 2022-08-18 01:57    [W:4.681 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site