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

[Dave Hansen]

On 2/23/26 04:58, Lance Yang wrote:
...
> +/**
> + * 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.

Just a nit on comments: Use imperative voice:

	... Use 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.

Yep, synchronize_rcu() is likely slower (longer wall clock time) but
less disruptive to other CPUs.

Is it worth explaining here that this should be used when code really
needs to _wait_ and *not* for freeing memory? Freeing memory should use
RCU callbacks that don't cause latency spikes in this thread, not this.

> + * Context: Can sleep/block. Cannot be called from any atomic context.

As a general rule, expressing constraints like this is best done in
code, not comments, so:

	might_sleep();
or
	WARN_ON_ONCE(!in_atomic());

seem appropriate.

I didn't see any obvious warning like that in the top levels of
synchronize_rcu().

> +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 */
This seems a _little_ dangerous to even define. We don't want this
sneaking into use when it doesn't do anything.