linux-kernel - Re: [PATCH v2 2/7] radix-tree: Use local

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200525062954.GA3180782@gmail.com>
Date:   Mon, 25 May 2020 08:29:54 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Will Deacon <will@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Matthew Wilcox <willy@...radead.org>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 2/7] radix-tree: Use local_lock for protection


* Sebastian Andrzej Siewior <bigeasy@...utronix.de> 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@...radead.org>
> Cc: linux-fsdevel@...r.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>  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