Re: [PATCH v2 2/7] radix-tree: Use local_lock for protection

From: Ingo Molnar
Date: Mon May 25 2020 - 02:30:01 EST



* Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:

> The radix-tree and idr preload mechanisms use preempt_disable() to protect
> the complete operation between xxx_preload() and xxx_preload_end().
>
> As the code inside the preempt disabled section acquires regular spinlocks,
> which are converted to 'sleeping' spinlocks on a PREEMPT_RT kernel and
> eventually calls into a memory allocator, this conflicts with the RT
> semantics.
>
> Convert it to a local_lock which allows RT kernels to substitute them with
> a real per CPU lock. On non RT kernels this maps to preempt_disable() as
> before, but provides also lockdep coverage of the critical region.
> No functional change.
>
> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Cc: linux-fsdevel@xxxxxxxxxxxxxxx
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> include/linux/idr.h | 5 +----
> include/linux/radix-tree.h | 6 +-----
> lib/radix-tree.c | 29 ++++++++++++++++++++++-------
> 3 files changed, 24 insertions(+), 16 deletions(-)

> -static inline void idr_preload_end(void)
> -{
> - preempt_enable();
> -}
> +void idr_preload_end(void);

> +void idr_preload_end(void)
> +{
> + local_unlock(&radix_tree_preloads.lock);
> +}
> +EXPORT_SYMBOL(idr_preload_end);

> +void radix_tree_preload_end(void);

> -static inline void radix_tree_preload_end(void)
> -{
> - preempt_enable();
> -}

> +void radix_tree_preload_end(void)
> +{
> + local_unlock(&radix_tree_preloads.lock);
> +}
> +EXPORT_SYMBOL(radix_tree_preload_end);

Since upstream we are still mapping the local_lock primitives to
preempt_disable()/preempt_enable(), I believe these uninlining changes should not be done
in this patch, i.e. idr_preload_end() and radix_tree_preload_end() should stay inline.

Thanks,

Ingo