Re: [PATCH/RFC] locking/spinlocks: Make __raw_* lock ops static

Waiman Long <[email protected]>
Newsgroups gmane.linux.ports.sh.devel,gmane.linux.kernel
Message-ID <[email protected]>
On 3/1/24 15:43, Geert Uytterhoeven wrote:
> sh/sdk7786_defconfig (CONFIG_GENERIC_LOCKBREAK=y and
> CONFIG_DEBUG_LOCK_ALLOC=n):
>
> kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_spin_lock' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_spin_lock_irqsave' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_spin_lock_irq' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_spin_lock_bh' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_read_lock' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_read_lock_irqsave' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_read_lock_irq' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_read_lock_bh' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_write_lock' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_write_lock_irqsave' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_write_lock_irq' [-Wmissing-prototypes]
> kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_write_lock_bh' [-Wmissing-prototypes]
>
> Fix this by making the __raw_* lock ops static.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Compile-tested only.
>
> Is SH really the only SMP platform where CONFIG_GENERIC_LOCKBREAK=y?
> ---
>   kernel/locking/spinlock.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/locking/spinlock.c b/kernel/locking/spinlock.c
> index 8475a0794f8c5ad2..7009b568e6255d64 100644
> --- a/kernel/locking/spinlock.c
> +++ b/kernel/locking/spinlock.c
> @@ -65,7 +65,7 @@ EXPORT_PER_CPU_SYMBOL(__mmiowb_state);
>    * towards that other CPU that it should break the lock ASAP.
>    */
>   #define BUILD_LOCK_OPS(op, locktype)					\
> -void __lockfunc __raw_##op##_lock(locktype##_t *lock)			\
> +static void __lockfunc __raw_##op##_lock(locktype##_t *lock)		\
>   {									\
>   	for (;;) {							\
>   		preempt_disable();					\
> @@ -77,7 +77,7 @@ void __lockfunc __raw_##op##_lock(locktype##_t *lock)			\
>   	}								\
>   }									\
>   									\
> -unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock)	\
> +static unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \
>   {									\
>   	unsigned long flags;						\
>   									\
> @@ -95,12 +95,12 @@ unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock)	\
>   	return flags;							\
>   }									\
>   									\
> -void __lockfunc __raw_##op##_lock_irq(locktype##_t *lock)		\
> +static void __lockfunc __raw_##op##_lock_irq(locktype##_t *lock)	\
>   {									\
>   	_raw_##op##_lock_irqsave(lock);					\
>   }									\
>   									\
> -void __lockfunc __raw_##op##_lock_bh(locktype##_t *lock)		\
> +static void __lockfunc __raw_##op##_lock_bh(locktype##_t *lock)		\
>   {									\
>   	unsigned long flags;						\
>   									\

This may not work if CONFIG_GENERIC_LOCKBREAK is defined. We had been 
talking about taking out CONFIG_GENERIC_LOCKBREAK before. See the thread 
in [1]. However, we didn't proceed further at that time as we weren't 
totally sure if there were still some configurations that required 
CONFIG_GENERIC_LOCKBREAK.

[1] https://lore.kernel.org/lkml/[email protected]/

Anyway, without taking out CONFIG_GENERIC_LOCKBREAK, the proper way to 
fix this issue is probably to declare the proper function prototypes in 
include/linux/rwlock_api_smp.h and include/linux/spinlock_api_smp.h when 
CONFIG_GENERIC_LOCKBREAK is defined.

Cheers,
Longman
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.