[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9e5c199a-9b4d-4d1b-97d4-dd2b776ac85f@huawei.com>
Date: Fri, 1 Dec 2023 19:09:29 +0800
From: Kefeng Wang <wangkefeng.wang@...wei.com>
To: Muchun Song <songmuchun@...edance.com>, <mike.kravetz@...cle.com>,
<muchun.song@...ux.dev>, <akpm@...ux-foundation.org>
CC: <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] mm: pagewalk: assert write mmap lock only for walking
the user page tables
On 2023/11/27 16:46, Muchun Song wrote:
> The 8782fb61cc848 ("mm: pagewalk: Fix race between unmap and page walker")
> introduces an assertion to walk_page_range_novma() to make all the users
> of page table walker is safe. However, the race only exists for walking the
> user page tables. And it is ridiculous to hold a particular user mmap write
> lock against the changes of the kernel page tables. So only assert at least
> mmap read lock when walking the kernel page tables. And some users matching
> this case could downgrade to a mmap read lock to relief the contention of
> mmap lock of init_mm, it will be nicer in hugetlb (only holding mmap read
> lock) in the next patch.
>
> Signed-off-by: Muchun Song <songmuchun@...edance.com>
> ---
> mm/pagewalk.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index b7d7e4fcfad7a..f46c80b18ce4f 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -539,6 +539,11 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
> * not backed by VMAs. Because 'unusual' entries may be walked this function
> * will also not lock the PTEs for the pte_entry() callback. This is useful for
> * walking the kernel pages tables or page tables for firmware.
> + *
> + * Note: Be careful to walk the kernel pages tables, the caller may be need to
> + * take other effective approache (mmap lock may be insufficient) to prevent
> + * the intermediate kernel page tables belonging to the specified address range
> + * from being freed (e.g. memory hot-remove).
> */
> int walk_page_range_novma(struct mm_struct *mm, unsigned long start,
> unsigned long end, const struct mm_walk_ops *ops,
> @@ -556,7 +561,29 @@ int walk_page_range_novma(struct mm_struct *mm, unsigned long start,
> if (start >= end || !walk.mm)
> return -EINVAL;
>
> - mmap_assert_write_locked(walk.mm);
> + /*
> + * 1) For walking the user virtual address space:
> + *
> + * The mmap lock protects the page walker from changes to the page
> + * tables during the walk. However a read lock is insufficient to
> + * protect those areas which don't have a VMA as munmap() detaches
> + * the VMAs before downgrading to a read lock and actually tearing
> + * down PTEs/page tables. In which case, the mmap write lock should
> + * be hold.
> + *
> + * 2) For walking the kernel virtual address space:
> + *
> + * The kernel intermediate page tables usually do not be freed, so
> + * the mmap map read lock is sufficient. But there are some exceptions.
> + * E.g. memory hot-remove. In which case, the mmap lock is insufficient
> + * to prevent the intermediate kernel pages tables belonging to the
> + * specified address range from being freed. The caller should take
> + * other actions to prevent this race.
> + */
> + if (mm == &init_mm)
> + mmap_assert_locked(walk.mm);
> + else
> + mmap_assert_write_locked(walk.mm);
Maybe just use process_mm_walk_lock() and set correct page_walk_lock in
struct mm_walk_ops?
>
> return walk_pgd_range(start, end, &walk);
> }
Powered by blists - more mailing lists