diff mbox series

[12/12] mm: change the contract for free_pgtables(), update docs

Message ID 20260901-rcu-pagetable-freeing-v1-12-5456a81c8212@kernel.org
State New
Headers show
Series mm: make userland page table freeing RCU-safe | expand

Commit Message

Lorenzo Stoakes (ARM) Sept. 1, 2026, 11:01 a.m. UTC
Now that page tables are freed after an RCU grace period, it is safe for
page table walkers to walk page table ranges that are being concurrently
torn down, provided the mm is kept alive via mmgrab().

The comment block before pte_offset_map_lock() established a contract that
this was unsafe, which was correct prior to these changes. Update it to
reflect the change.

Similarly update the process addresses documentation.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 Documentation/mm/process_addrs.rst |  6 ++++++
 mm/pgtable-generic.c               | 18 +++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

Comments

Kiryl Shutsemau Sept. 1, 2026, 1:57 p.m. UTC | #1
On Tue, Sep 01, 2026 at 12:01:32PM +0100, Lorenzo Stoakes (ARM) wrote:
> diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> index b91b1a98029c..ff8ff3706485 100644
> --- a/mm/pgtable-generic.c
> +++ b/mm/pgtable-generic.c
> @@ -386,9 +386,21 @@ pte_t *pte_offset_map_rw_nolock(struct mm_struct *mm, pmd_t *pmd,
>   * be read-only/read-write protected.
>   *
>   * Note that free_pgtables(), used after unmapping detached vmas, or when
> - * exiting the whole mm, does not take page table lock before freeing a page
> - * table, and may not use RCU at all: "outsiders" like khugepaged should avoid
> - * pte_offset_map() and co once the vma is detached from mm or mm_users is zero.
> + * exiting the whole mm, does not take the page table lock before freeing a
> + * table.
> + *
> + * However, the PMD entry is cleared first, and the table freed only after
> + * an RCU grace period, so a walker that mapped the table under
> + * rcu_read_lock() stays safe, and the pmd_same() recheck in
> + * pte_offset_map_lock() detects the teardown.
> + *
> + * Therefore it is safe for "outsiders" like khugepaged to use
> + * pte_offset_map() and co. for VMAs that might be undergoing page table
> + * teardown.

Is it strictly true?

free_pte_range() clears the pmd without taking the PTL, so the
pmd_same() recheck in pte_offset_map_lock() can pass and the pmd gets
cleared right after.

Readers are fine -- __pte_offset_map() holds rcu_read_lock() until
pte_unmap(), so the table cannot go away.

khugepaged is an odd example here. collapse_pte_mapped_thp() and
retract_page_tables() don't rely on this recheck -- they take pmd_lock()
plus the ptl and do their own pmd_same() under both.

Can we say walks are safe and leave the write rule where it is?
Lorenzo Stoakes (ARM) Sept. 1, 2026, 2:31 p.m. UTC | #2
On Tue, Sep 01, 2026 at 02:57:13PM +0100, Kiryl Shutsemau wrote:
> On Tue, Sep 01, 2026 at 12:01:32PM +0100, Lorenzo Stoakes (ARM) wrote:
> > diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> > index b91b1a98029c..ff8ff3706485 100644
> > --- a/mm/pgtable-generic.c
> > +++ b/mm/pgtable-generic.c
> > @@ -386,9 +386,21 @@ pte_t *pte_offset_map_rw_nolock(struct mm_struct *mm, pmd_t *pmd,
> >   * be read-only/read-write protected.
> >   *
> >   * Note that free_pgtables(), used after unmapping detached vmas, or when
> > - * exiting the whole mm, does not take page table lock before freeing a page
> > - * table, and may not use RCU at all: "outsiders" like khugepaged should avoid
> > - * pte_offset_map() and co once the vma is detached from mm or mm_users is zero.
> > + * exiting the whole mm, does not take the page table lock before freeing a
> > + * table.
> > + *
> > + * However, the PMD entry is cleared first, and the table freed only after
> > + * an RCU grace period, so a walker that mapped the table under
> > + * rcu_read_lock() stays safe, and the pmd_same() recheck in
> > + * pte_offset_map_lock() detects the teardown.
> > + *
> > + * Therefore it is safe for "outsiders" like khugepaged to use
> > + * pte_offset_map() and co. for VMAs that might be undergoing page table
> > + * teardown.
>
> Is it strictly true?
>
> free_pte_range() clears the pmd without taking the PTL, so the
> pmd_same() recheck in pte_offset_map_lock() can pass and the pmd gets
> cleared right after.
>
> Readers are fine -- __pte_offset_map() holds rcu_read_lock() until
> pte_unmap(), so the table cannot go away.
>
> khugepaged is an odd example here. collapse_pte_mapped_thp() and
> retract_page_tables() don't rely on this recheck -- they take pmd_lock()
> plus the ptl and do their own pmd_same() under both.
>
> Can we say walks are safe and leave the write rule where it is?

Hmm yeah, ah page tables, what a rabbit hole of horror :)

I guess this is effectively a reflection of the 'write lock on write, RCU load
on read' pattern in RCU itself generally.

So, maybe something like:

 * Note that free_pgtables(), used after unampping detached vmas, or when
 * exiting the whoel mm, does not take a page table lock before freeing a page
 * table.
 *
 * As page table freeing itself is RCU-safe, page table readers can safely run
 * concurrently with page table teardown. However, writers cannot, as without
 * a lock nothing prevents concurrent teardown.
 */

Instead? What do you think?

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

--
Cheers, Lorenzo
Kiryl Shutsemau Sept. 1, 2026, 5:15 p.m. UTC | #3
On Tue, Sep 01, 2026 at 03:31:26PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Tue, Sep 01, 2026 at 02:57:13PM +0100, Kiryl Shutsemau wrote:
> > On Tue, Sep 01, 2026 at 12:01:32PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> > > index b91b1a98029c..ff8ff3706485 100644
> > > --- a/mm/pgtable-generic.c
> > > +++ b/mm/pgtable-generic.c
> > > @@ -386,9 +386,21 @@ pte_t *pte_offset_map_rw_nolock(struct mm_struct *mm, pmd_t *pmd,
> > >   * be read-only/read-write protected.
> > >   *
> > >   * Note that free_pgtables(), used after unmapping detached vmas, or when
> > > - * exiting the whole mm, does not take page table lock before freeing a page
> > > - * table, and may not use RCU at all: "outsiders" like khugepaged should avoid
> > > - * pte_offset_map() and co once the vma is detached from mm or mm_users is zero.
> > > + * exiting the whole mm, does not take the page table lock before freeing a
> > > + * table.
> > > + *
> > > + * However, the PMD entry is cleared first, and the table freed only after
> > > + * an RCU grace period, so a walker that mapped the table under
> > > + * rcu_read_lock() stays safe, and the pmd_same() recheck in
> > > + * pte_offset_map_lock() detects the teardown.
> > > + *
> > > + * Therefore it is safe for "outsiders" like khugepaged to use
> > > + * pte_offset_map() and co. for VMAs that might be undergoing page table
> > > + * teardown.
> >
> > Is it strictly true?
> >
> > free_pte_range() clears the pmd without taking the PTL, so the
> > pmd_same() recheck in pte_offset_map_lock() can pass and the pmd gets
> > cleared right after.
> >
> > Readers are fine -- __pte_offset_map() holds rcu_read_lock() until
> > pte_unmap(), so the table cannot go away.
> >
> > khugepaged is an odd example here. collapse_pte_mapped_thp() and
> > retract_page_tables() don't rely on this recheck -- they take pmd_lock()
> > plus the ptl and do their own pmd_same() under both.
> >
> > Can we say walks are safe and leave the write rule where it is?
> 
> Hmm yeah, ah page tables, what a rabbit hole of horror :)
> 
> I guess this is effectively a reflection of the 'write lock on write, RCU load
> on read' pattern in RCU itself generally.
> 
> So, maybe something like:
> 
>  * Note that free_pgtables(), used after unampping detached vmas, or when
>  * exiting the whoel mm, does not take a page table lock before freeing a page
>  * table.
>  *
>  * As page table freeing itself is RCU-safe, page table readers can safely run
>  * concurrently with page table teardown. However, writers cannot, as without
>  * a lock nothing prevents concurrent teardown.
>  */
> 
> Instead? What do you think?

s/whoel/whole/

Otherwise, LGTM.
Lorenzo Stoakes (ARM) Sept. 1, 2026, 5:25 p.m. UTC | #4
On Tue, Sep 01, 2026 at 06:15:17PM +0100, Kiryl Shutsemau wrote:
> On Tue, Sep 01, 2026 at 03:31:26PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Tue, Sep 01, 2026 at 02:57:13PM +0100, Kiryl Shutsemau wrote:
> > > On Tue, Sep 01, 2026 at 12:01:32PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > > diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
> > > > index b91b1a98029c..ff8ff3706485 100644
> > > > --- a/mm/pgtable-generic.c
> > > > +++ b/mm/pgtable-generic.c
> > > > @@ -386,9 +386,21 @@ pte_t *pte_offset_map_rw_nolock(struct mm_struct *mm, pmd_t *pmd,
> > > >   * be read-only/read-write protected.
> > > >   *
> > > >   * Note that free_pgtables(), used after unmapping detached vmas, or when
> > > > - * exiting the whole mm, does not take page table lock before freeing a page
> > > > - * table, and may not use RCU at all: "outsiders" like khugepaged should avoid
> > > > - * pte_offset_map() and co once the vma is detached from mm or mm_users is zero.
> > > > + * exiting the whole mm, does not take the page table lock before freeing a
> > > > + * table.
> > > > + *
> > > > + * However, the PMD entry is cleared first, and the table freed only after
> > > > + * an RCU grace period, so a walker that mapped the table under
> > > > + * rcu_read_lock() stays safe, and the pmd_same() recheck in
> > > > + * pte_offset_map_lock() detects the teardown.
> > > > + *
> > > > + * Therefore it is safe for "outsiders" like khugepaged to use
> > > > + * pte_offset_map() and co. for VMAs that might be undergoing page table
> > > > + * teardown.
> > >
> > > Is it strictly true?
> > >
> > > free_pte_range() clears the pmd without taking the PTL, so the
> > > pmd_same() recheck in pte_offset_map_lock() can pass and the pmd gets
> > > cleared right after.
> > >
> > > Readers are fine -- __pte_offset_map() holds rcu_read_lock() until
> > > pte_unmap(), so the table cannot go away.
> > >
> > > khugepaged is an odd example here. collapse_pte_mapped_thp() and
> > > retract_page_tables() don't rely on this recheck -- they take pmd_lock()
> > > plus the ptl and do their own pmd_same() under both.
> > >
> > > Can we say walks are safe and leave the write rule where it is?
> >
> > Hmm yeah, ah page tables, what a rabbit hole of horror :)
> >
> > I guess this is effectively a reflection of the 'write lock on write, RCU load
> > on read' pattern in RCU itself generally.
> >
> > So, maybe something like:
> >
> >  * Note that free_pgtables(), used after unampping detached vmas, or when
> >  * exiting the whoel mm, does not take a page table lock before freeing a page
> >  * table.
> >  *
> >  * As page table freeing itself is RCU-safe, page table readers can safely run
> >  * concurrently with page table teardown. However, writers cannot, as without
> >  * a lock nothing prevents concurrent teardown.
> >  */
> >
> > Instead? What do you think?
>
> s/whoel/whole/

Haha yup the inevitable typo...

>
> Otherwise, LGTM.

Thanks! :)

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

--
Cheers, Lorenzo
diff mbox series

Patch

diff --git a/Documentation/mm/process_addrs.rst b/Documentation/mm/process_addrs.rst
index a7296f251799..1e65b139f355 100644
--- a/Documentation/mm/process_addrs.rst
+++ b/Documentation/mm/process_addrs.rst
@@ -537,6 +537,12 @@  We establish basic locking rules when interacting with page tables:
 * When changing a page table entry the page table lock for that page table
   **must** be held, except if you can safely assume nobody can access the page
   tables concurrently (such as on invocation of :c:func:`!free_pgtables`).
+* Page tables may be *walked* under RCU alone, as page tables are freed only
+  after an RCU grace period has elapsed. However, any entry found must be
+  revalidated after the page table lock is taken (such as the
+  :c:func:`!pmd_same` recheck performed by :c:func:`!pte_offset_map_lock`)
+  before it is acted upon. Changing an entry always requires the page table
+  lock.
 * Reads from and writes to page table entries must be *appropriately*
   atomic. See the section on atomicity below for details.
 * Populating previously empty entries requires that the mmap or VMA locks are
diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
index b91b1a98029c..ff8ff3706485 100644
--- a/mm/pgtable-generic.c
+++ b/mm/pgtable-generic.c
@@ -386,9 +386,21 @@  pte_t *pte_offset_map_rw_nolock(struct mm_struct *mm, pmd_t *pmd,
  * be read-only/read-write protected.
  *
  * Note that free_pgtables(), used after unmapping detached vmas, or when
- * exiting the whole mm, does not take page table lock before freeing a page
- * table, and may not use RCU at all: "outsiders" like khugepaged should avoid
- * pte_offset_map() and co once the vma is detached from mm or mm_users is zero.
+ * exiting the whole mm, does not take the page table lock before freeing a
+ * table.
+ *
+ * However, the PMD entry is cleared first, and the table freed only after
+ * an RCU grace period, so a walker that mapped the table under
+ * rcu_read_lock() stays safe, and the pmd_same() recheck in
+ * pte_offset_map_lock() detects the teardown.
+ *
+ * Therefore it is safe for "outsiders" like khugepaged to use
+ * pte_offset_map() and co. for VMAs that might be undergoing page table
+ * teardown.
+ *
+ * Note that the PGD itself is freed at mmdrop() time, not under RCU - so the
+ * walker must keep the mm alive via mmgrab(). With that held, walking remains
+ * safe even once mm_users has reached zero.
  */
 pte_t *pte_offset_map_lock(struct mm_struct *mm, pmd_t *pmd,
 			   unsigned long addr, spinlock_t **ptlp)