Re: [PATCH RFC 00/10] mm/hugetlb: Make huge_pte_offset() thread-safe for pmd unshare

[Mike Kravetz]

On 10/30/22 17:29, Peter Xu wrote:
> Resolution
> ==========
> 
> What this patch proposed is, besides using the vma lock, we can also use
> RCU to protect the pgtable page from being freed from under us when
> huge_pte_offset() is used.  The idea is kind of similar to RCU fast-gup.
> Note that fast-gup is very safe regarding pmd unsharing even before vma
> lock, because fast-gup relies on RCU to protect walking any pgtable page,
> including another mm's.
> 
> To apply the same idea to huge_pte_offset(), it means with proper RCU
> protection the pte_t* pointer returned from huge_pte_offset() can also be
> always safe to access and de-reference, along with the pgtable lock that
> was bound to the pgtable page.
> 
> Patch Layout
> ============
> 
> Patch 1 is a trivial cleanup that I noticed when working on this.  Please
> shoot if anyone think I should just post it separately, or hopefully I can
> still just carry it over.
> 
> Patch 2 is the gut of the patchset, describing how we should use the helper
> huge_pte_offset() correctly. Only a comment patch but should be the most
> important one, as the follow up patches are just trying to follow the rule
> it setup here.
> 
> The rest patches resolve all the call sites of huge_pte_offset() to make
> sure either it's with the vma lock (which is perfectly good enough for
> safety in this case; the last patch commented on all those callers to make
> sure we won't miss a single case, and why they're safe).  Besides, each of
> the patch will add rcu protection to one caller of huge_pte_offset().
> 
> Tests
> =====
> 
> Only lightly tested on hugetlb kselftests including uffd, no more errors
> triggered than current mm-unstable (hugetlb-madvise fails before/after
> here, with error "Unexpected number of free huge pages line 207"; haven't
> really got time to look into it).

Do not worry about the madvise test failure, that is caused by a recent
change.

Unless I am missing something, the basic strategy in this series is to
wrap calls to huge_pte_offset and subsequent ptep access with
rcu_read_lock/unlock calls.  I must embarrassingly admit that it has
been a loooong time since I had to look at rcu usage and may not know
what I am talking about.  However, I seem to recall that one needs to
somehow flag the data items being protected from update/freeing.  I
do not see anything like that in the huge_pmd_unshare routine where
pmd page pointer is updated.  Or, is it where the pmd page pointer is
referenced in huge_pte_offset?

Please ignore if you are certain of this rcu usage, otherwise I will
spend some time reeducating myself.
-- 
Mike Kravetz