Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Nov 2022 16:51:21 -0400
From:   Peter Xu <peterx@redhat.com>
To:     Nadav Amit <nadav.amit@gmail.com>
Cc:     kernel list <linux-kernel@vger.kernel.org>, linux-mm@kvack.org,
        Andrew Morton <akpm@linux-foundation.org>,
        James Houghton <jthoughton@google.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        David Hildenbrand <david@redhat.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Rik van Riel <riel@surriel.com>
Subject: Re: [PATCH RFC 02/10] mm/hugetlb: Comment huge_pte_offset() for its
 locking requirements
Message-ID: <Y2LYSYLj94ajCaOI@x1n>
References: <20221030212929.335473-1-peterx@redhat.com>
 <20221030212929.335473-3-peterx@redhat.com>
 <F09923E5-605F-476E-976F-C7EABDF64938@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <F09923E5-605F-476E-976F-C7EABDF64938@gmail.com>
Precedence: bulk

On Mon, Oct 31, 2022 at 10:46:52PM -0700, Nadav Amit wrote:
> On Oct 30, 2022, at 2:29 PM, Peter Xu <peterx@redhat.com> wrote:
> 
> > huge_pte_offset() is potentially a pgtable walker, looking up pte_t* for a
> > hugetlb address.
> > 
> > Normally, it's always safe to walk the pgtable as long as we're with the
> > mmap lock held for either read or write, because that guarantees the
> > pgtable pages will always be valid during the process.
> > 
> > But it's not true for hugetlbfs: hugetlbfs has the pmd sharing feature, it
> > means that even with mmap lock held, the PUD pgtable page can still go away
> > from under us if pmd unsharing is possible during the walk.
> > 
> > It's not always the case, e.g.:
> > 
> >  (1) If the mapping is private we're not prone to pmd sharing or
> >      unsharing, so it's okay.
> > 
> >  (2) If we're with the hugetlb vma lock held for either read/write, it's
> >      okay too because pmd unshare cannot happen at all.
> > 
> > Document all these explicitly for huge_pte_offset(), because it's really
> > not that obvious.  This also tells all the callers on what it needs to
> > guarantee huge_pte_offset() thread-safety.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > arch/arm64/mm/hugetlbpage.c | 32 ++++++++++++++++++++++++++++++++
> 
> Please excuse my ignorant question - is there something specific for arm64
> code here? Other archs seem to have similar code, no?

Oops, sorry, I meant to add this to the version in mm/hugetlb.c.  Or maybe
to linux/hugetlb.h would make more sense.

I should probably also mention that for any arch that does not support pmd
sharing at all (afaik, any arch outside arm, x86 and riscv), the shared
mapping locking rule should be the same as private.

-- 
Peter Xu