Re: [PATCH bpf-next v8 3/6] locking/local_lock: Introduce localtry_lock_t

[Vlastimil Babka <vbabka@suse.cz>]

On 2/13/25 04:35, Alexei Starovoitov wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> In !PREEMPT_RT local_lock_irqsave() disables interrupts to protect
> critical section, but it doesn't prevent NMI, so the fully reentrant
> code cannot use local_lock_irqsave() for exclusive access.
> 
> Introduce localtry_lock_t and localtry_lock_irqsave() that
> disables interrupts and sets acquired=1, so localtry_lock_irqsave()
> from NMI attempting to acquire the same lock will return false.
> 
> In PREEMPT_RT local_lock_irqsave() maps to preemptible spin_lock().
> Map localtry_lock_irqsave() to preemptible spin_trylock().
> When in hard IRQ or NMI return false right away, since
> spin_trylock() is not safe due to PI issues.
> 
> Note there is no need to use local_inc for acquired variable,
> since it's a percpu variable with strict nesting scopes.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  include/linux/local_lock.h          |  59 +++++++++++++
>  include/linux/local_lock_internal.h | 123 ++++++++++++++++++++++++++++
>  2 files changed, 182 insertions(+)
> 
> diff --git a/include/linux/local_lock.h b/include/linux/local_lock.h
> index 091dc0b6bdfb..05c254a5d7d3 100644
> --- a/include/linux/local_lock.h
> +++ b/include/linux/local_lock.h
> @@ -51,6 +51,65 @@
>  #define local_unlock_irqrestore(lock, flags)			\
>  	__local_unlock_irqrestore(lock, flags)
>  
> +/**
> + * localtry_lock_init - Runtime initialize a lock instance
> + */
> +#define localtry_lock_init(lock)		__localtry_lock_init(lock)
> +
> +/**
> + * localtry_lock - Acquire a per CPU local lock
> + * @lock:	The lock variable
> + */
> +#define localtry_lock(lock)		__localtry_lock(lock)
> +
> +/**
> + * localtry_lock_irq - Acquire a per CPU local lock and disable interrupts
> + * @lock:	The lock variable
> + */
> +#define localtry_lock_irq(lock)		__localtry_lock_irq(lock)
> +
> +/**
> + * localtry_lock_irqsave - Acquire a per CPU local lock, save and disable
> + *			 interrupts
> + * @lock:	The lock variable
> + * @flags:	Storage for interrupt flags
> + */
> +#define localtry_lock_irqsave(lock, flags)				\
> +	__localtry_lock_irqsave(lock, flags)
> +
> +/**
> + * localtry_trylock_irqsave - Try to acquire a per CPU local lock, save and disable
> + *			      interrupts if acquired
> + * @lock:	The lock variable
> + * @flags:	Storage for interrupt flags
> + *
> + * The function can be used in any context such as NMI or HARDIRQ. Due to
> + * locking constrains it will _always_ fail to acquire the lock on PREEMPT_RT.

The "always fail" applies only to the NMI and HARDIRQ contexts, right? It's
not entirely obvious so it sounds worse than it is.

> +
> +#define __localtry_trylock_irqsave(lock, flags)			\
> +	({							\
> +		int __locked;					\
> +								\
> +		typecheck(unsigned long, flags);		\
> +		flags = 0;					\
> +		if (in_nmi() | in_hardirq()) {			\
> +			__locked = 0;				\

Because of this, IIUC?

> +		} else {					\
> +			migrate_disable();			\
> +			__locked = spin_trylock(this_cpu_ptr((lock)));	\
> +			if (!__locked)				\
> +				migrate_enable();		\
> +		}						\
> +		__locked;					\
> +	})
> +
>  #endif /* CONFIG_PREEMPT_RT */