Re: [PATCH 1/1] mm/mmu_gather: replace IPI with synchronize_rcu() when batch allocation fails

["David Hildenbrand (Arm)" <david@kernel.org>]

On 2/23/26 13:58, Lance Yang wrote:
> 
> On Mon, Feb 23, 2026 at 10:29:56AM +0100, David Hildenbrand (Arm) wrote:
>> On 2/23/26 04:36, Lance Yang wrote:
>>> From: Lance Yang <lance.yang@linux.dev>
>>>
>>> When freeing page tables, we try to batch them. If batch allocation fails
>>> (GFP_NOWAIT), __tlb_remove_table_one() immediately frees the one without
>>> batching.
>>>
>>> On !CONFIG_PT_RECLAIM, the fallback sends an IPI to all CPUs via
>>> tlb_remove_table_sync_one(). It disrupts all CPUs even when only a single
>>> process is unmapping memory. IPI broadcast was reported to hurt RT
>>> workloads[1].
>>>
>>> tlb_remove_table_sync_one() synchronizes with lockless page-table walkers
>>> (e.g. GUP-fast) that rely on IRQ disabling. These walkers use
>>> local_irq_disable(), which is also an RCU read-side critical section.
>>> synchronize_rcu() waits for all such sections to complete, providing the
>>> same guarantee as IPI but without disrupting all CPUs.
>>>
>>> Since batch allocation already failed, we are in a way slow path, so
>>> replacing the IPI with synchronize_rcu() is fine.
>>>
>>> We are in process context (unmap_region, exit_mmap) with only mmap_lock
>>> held, a sleeping lock. synchronize_rcu() will catch any invalid context
>>> via might_sleep().
>>>
>>> [1] https://lore.kernel.org/linux-mm/1b27a3fa-359a-43d0-bdeb-c31341749367@kernel.org/
>>>
>>> Link: https://lore.kernel.org/linux-mm/20260202150957.GD1282955@noisy.programming.kicks-ass.net/
>>> Link: https://lore.kernel.org/linux-mm/dfdfeac9-5cd5-46fc-a5c1-9ccf9bd3502a@intel.com/
>>> Link: https://lore.kernel.org/linux-mm/bc489455-bb18-44dc-8518-ae75abda6bec@kernel.org/
>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>> Suggested-by: Dave Hansen <dave.hansen@intel.com>
>>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>> I think it was primarily Peter and Dave suggesting that :)
> 
> :)
> 
>>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
>>> ---
>>>    mm/mmu_gather.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
>>> index fe5b6a031717..df670c219260 100644
>>> --- a/mm/mmu_gather.c
>>> +++ b/mm/mmu_gather.c
>>> @@ -339,7 +339,8 @@ static inline void __tlb_remove_table_one(void *table)
>>>    #else
>>>    static inline void __tlb_remove_table_one(void *table)
>>>    {
>>> -	tlb_remove_table_sync_one();
>>> +	if (IS_ENABLED(CONFIG_MMU_GATHER_RCU_TABLE_FREE))
>>> +		synchronize_rcu();
>>
>> That should work.
>>
>> Reading all the comments for tlb_remove_table_smp_sync(), I wonder
>> whether we should wrap that in a tlb_remove_table_sync_rcu() function,
>> with a proper kerneldoc for the CONFIG_MMU_GATHER_RCU_TABLE_FREE variant
>> where we discuss how this relates to tlb_remove_table_sync_one (and
>> tlb_remove_table_smp_sync() .
> 
> Good point! That would be cleaner and better ;)
> 
> How about the following:
> 
> ---8<---
> diff --git a/mm/mmu_gather.c b/mm/mmu_gather.c
> index fe5b6a031717..ea5503d3e650 100644
> --- a/mm/mmu_gather.c
> +++ b/mm/mmu_gather.c
> @@ -296,6 +296,24 @@ static void tlb_remove_table_free(struct mmu_table_batch *batch)
>   	call_rcu(&batch->rcu, tlb_remove_table_rcu);
>   }
> 
> +/**
> + * tlb_remove_table_sync_rcu() - synchronize with software page-table walkers
> + *
> + * Like tlb_remove_table_sync_one() but uses RCU grace period instead of IPI
> + * broadcast. Should be used in slow paths where sleeping is acceptable.
> + *
> + * Software/Lockless page-table walkers use local_irq_disable(), which is also
> + * an RCU read-side critical section. synchronize_rcu() waits for all such
> + * sections, providing the same guarantee as tlb_remove_table_sync_one() but
> + * without disrupting all CPUs with IPIs.
> + *
> + * Context: Can sleep/block. Cannot be called from any atomic context.
> + */
> +static void tlb_remove_table_sync_rcu(void)
> +{
> +	synchronize_rcu();
> +}
> +
>   #else /* !CONFIG_MMU_GATHER_RCU_TABLE_FREE */
> 
>   static void tlb_remove_table_free(struct mmu_table_batch *batch)
> @@ -303,6 +321,10 @@ static void tlb_remove_table_free(struct mmu_table_batch *batch)
>   	__tlb_remove_table_free(batch);
>   }
> 
> +static void tlb_remove_table_sync_rcu(void)
> +{
> +}
> +
>   #endif /* CONFIG_MMU_GATHER_RCU_TABLE_FREE */
> 
>   /*
> @@ -339,7 +361,7 @@ static inline void __tlb_remove_table_one(void *table)
>   #else
>   static inline void __tlb_remove_table_one(void *table)
>   {
> -	tlb_remove_table_sync_one();
> +	tlb_remove_table_sync_rcu();
>   	__tlb_remove_table(table);
>   }
>   #endif /* CONFIG_PT_RECLAIM */
> ---
> 
> Thanks for the suggestion!
> Lance

LGTM, but let's hear other options.


-- 
Cheers,

David