[v9,0/3] fix hugetlb MADV_DONTNEED vma_lock handling

Message

Mike Kravetz Nov. 11, 2022, 11:26 p.m. UTC

This series addresses the issue first reported in [1], and fully
described in patch 3.  While exploring solutions to this issue,
related problems with mmu notification calls were discovered.  The
first two patches address those issues.

Previous discussions suggested further cleanup by removing the
routine zap_page_range.  This is possible because zap_page_range_single
is now exported, and all callers of zap_page_range pass ranges entirely
within a single vma.  This work will be done in a later patch so as not
to distract from this bug fix.

[1] https://lore.kernel.org/lkml/[e-mail]/

Mike Kravetz (3):
  madvise: use zap_page_range_single for madvise dontneed
  hugetlb: remove duplicate mmu notifications
  hugetlb: don't delete vma_lock in hugetlb MADV_DONTNEED processing

 include/linux/mm.h | 29 +++++++++++++++++++++--------
 mm/hugetlb.c       | 45 +++++++++++++++++++++++++--------------------
 mm/madvise.c       |  6 +++---
 mm/memory.c        | 25 ++++++++++++-------------
 4 files changed, 61 insertions(+), 44 deletions(-)

Comments

Nadav Amit Nov. 12, 2022, 7:37 p.m. UTC | #1

On Nov 11, 2022, at 3:26 PM, Mike Kravetz <[e-mail]> wrote:

> This series addresses the issue first reported in [1], and fully
> described in patch 3.  While exploring solutions to this issue,
> related problems with mmu notification calls were discovered.  The
> first two patches address those issues.
> 
> Previous discussions suggested further cleanup by removing the
> routine zap_page_range.  This is possible because zap_page_range_single
> is now exported, and all callers of zap_page_range pass ranges entirely
> within a single vma.  This work will be done in a later patch so as not
> to distract from this bug fix.
> 
> [1] https://lore.kernel.org/lkml/[e-mail]/
> 
> Mike Kravetz (3):
>  madvise: use zap_page_range_single for madvise dontneed
>  hugetlb: remove duplicate mmu notifications
>  hugetlb: don't delete vma_lock in hugetlb MADV_DONTNEED processing
> 
> include/linux/mm.h | 29 +++++++++++++++++++++--------
> mm/hugetlb.c       | 45 +++++++++++++++++++++++++--------------------
> mm/madvise.c       |  6 +++---
> mm/memory.c        | 25 ++++++++++++-------------
> 4 files changed, 61 insertions(+), 44 deletions(-)

With my limited knowledge of hugetlbfs, it all looks good.

Having said that - 2 random thoughts:

1. It is more intuitive to me to have
mmu_notifier_invalidate_range_{start|end}() next to tlb_{start|end}_vma().
I think that one day these two should have been combined into a
single function, which could have also executed
adjust_range_if_pmd_sharing_possible() as needed.

2. If you still have a concern of exposing zap_details as you had in the
past (not that I care), consider putting zap_details and
zap_page_range_single() in mm/internal.h.

Thanks,
Nadav

Peter Xu Nov. 14, 2022, 12:09 a.m. UTC | #2

On Fri, Nov 11, 2022 at 03:26:25PM -0800, Mike Kravetz wrote:
> This series addresses the issue first reported in [1], and fully
> described in patch 3.  While exploring solutions to this issue,
> related problems with mmu notification calls were discovered.  The
> first two patches address those issues.
> 
> Previous discussions suggested further cleanup by removing the
> routine zap_page_range.  This is possible because zap_page_range_single
> is now exported, and all callers of zap_page_range pass ranges entirely
> within a single vma.  This work will be done in a later patch so as not
> to distract from this bug fix.
> 
> [1] https://lore.kernel.org/lkml/[e-mail]/
> 
> Mike Kravetz (3):
>   madvise: use zap_page_range_single for madvise dontneed
>   hugetlb: remove duplicate mmu notifications
>   hugetlb: don't delete vma_lock in hugetlb MADV_DONTNEED processing

Acked-by: Peter Xu <[e-mail]>