| Message ID | 20211115185909.3949505-8-minchan@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | zsmalloc: remove bit_spin_lock | expand |
On Mon, Nov 15, 2021 at 10:59:07AM -0800, Minchan Kim wrote: > In preparation for converting bit_spin_lock to rwlock in zsmalloc > so that multiple writers of zspages can run at the same time but > those zspages are supposed to be different zspage instance. Thus, > it's not deadlock. This patch adds write_lock_nested to support > the case for LOCKDEP. > > Cc: Peter Zijlstra (Intel) <peterz@infradead.org> > Signed-off-by: Minchan Kim <minchan@kernel.org> > --- > include/linux/rwlock.h | 6 ++++++ > include/linux/rwlock_api_smp.h | 9 +++++++++ > include/linux/rwlock_rt.h | 6 ++++++ > include/linux/spinlock_api_up.h | 1 + > kernel/locking/spinlock.c | 6 ++++++ > kernel/locking/spinlock_rt.c | 12 ++++++++++++ > 6 files changed, 40 insertions(+) Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
On 2021-11-15 10:59:07 [-0800], Minchan Kim wrote: > diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h > index 49c1f3842ed5..efd6da62c893 100644 > --- a/include/linux/rwlock_rt.h > +++ b/include/linux/rwlock_rt.h > @@ -28,6 +28,7 @@ extern void rt_read_lock(rwlock_t *rwlock); > extern int rt_read_trylock(rwlock_t *rwlock); > extern void rt_read_unlock(rwlock_t *rwlock); > extern void rt_write_lock(rwlock_t *rwlock); > +extern void rt_write_lock_nested(rwlock_t *rwlock, int subclass); > extern int rt_write_trylock(rwlock_t *rwlock); > extern void rt_write_unlock(rwlock_t *rwlock); > > @@ -83,6 +84,11 @@ static __always_inline void write_lock(rwlock_t *rwlock) > rt_write_lock(rwlock); > } > > +static __always_inline void write_lock_nested(rwlock_t *rwlock, int subclass) > +{ > + rt_write_lock_nested(rwlock, subclass); > +} > + These two hunks as-is don't work. You need a CONFIG_DEBUG_LOCK_ALLOC block and in the !CONFIG_DEBUG_LOCK_ALLOC case you need #define rt_write_lock_nested(lock, subclass) rt_write_lock(((void)(subclass), (lock))) > static __always_inline void write_lock_bh(rwlock_t *rwlock) > { > local_bh_disable(); > diff --git a/kernel/locking/spinlock_rt.c b/kernel/locking/spinlock_rt.c > index b2e553f9255b..b82d346f1e00 100644 > --- a/kernel/locking/spinlock_rt.c > +++ b/kernel/locking/spinlock_rt.c > @@ -239,6 +239,18 @@ void __sched rt_write_lock(rwlock_t *rwlock) > } > EXPORT_SYMBOL(rt_write_lock); > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > +void __sched rt_write_lock_nested(rwlock_t *rwlock, int subclass) > +{ > + ___might_sleep(__FILE__, __LINE__, 0); This _must_ be rtlock_might_resched() like it is done in rt_write_lock() above. > + rwlock_acquire(&rwlock->dep_map, subclass, 0, _RET_IP_); > + rwbase_write_lock(&rwlock->rwbase, TASK_RTLOCK_WAIT); > + rcu_read_lock(); > + migrate_disable(); > +} > +EXPORT_SYMBOL(rt_write_lock_nested); > +#endif > + > void __sched rt_read_unlock(rwlock_t *rwlock) > { > rwlock_release(&rwlock->dep_map, _RET_IP_); Sebastian
On Fri, Nov 19, 2021 at 11:35:16AM +0100, Sebastian Andrzej Siewior wrote: > On 2021-11-15 10:59:07 [-0800], Minchan Kim wrote: > > diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h > > index 49c1f3842ed5..efd6da62c893 100644 > > --- a/include/linux/rwlock_rt.h > > +++ b/include/linux/rwlock_rt.h > > @@ -28,6 +28,7 @@ extern void rt_read_lock(rwlock_t *rwlock); > > extern int rt_read_trylock(rwlock_t *rwlock); > > extern void rt_read_unlock(rwlock_t *rwlock); > > extern void rt_write_lock(rwlock_t *rwlock); > > +extern void rt_write_lock_nested(rwlock_t *rwlock, int subclass); > > extern int rt_write_trylock(rwlock_t *rwlock); > > extern void rt_write_unlock(rwlock_t *rwlock); > > > > @@ -83,6 +84,11 @@ static __always_inline void write_lock(rwlock_t *rwlock) > > rt_write_lock(rwlock); > > } > > > > +static __always_inline void write_lock_nested(rwlock_t *rwlock, int subclass) > > +{ > > + rt_write_lock_nested(rwlock, subclass); > > +} > > + > > These two hunks as-is don't work. You need a CONFIG_DEBUG_LOCK_ALLOC block and > in the !CONFIG_DEBUG_LOCK_ALLOC case you need > > #define rt_write_lock_nested(lock, subclass) rt_write_lock(((void)(subclass), (lock))) Guess you meant #define write_lock_nested. > > > static __always_inline void write_lock_bh(rwlock_t *rwlock) > > { > > local_bh_disable(); > > diff --git a/kernel/locking/spinlock_rt.c b/kernel/locking/spinlock_rt.c > > index b2e553f9255b..b82d346f1e00 100644 > > --- a/kernel/locking/spinlock_rt.c > > +++ b/kernel/locking/spinlock_rt.c > > @@ -239,6 +239,18 @@ void __sched rt_write_lock(rwlock_t *rwlock) > > } > > EXPORT_SYMBOL(rt_write_lock); > > > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > > +void __sched rt_write_lock_nested(rwlock_t *rwlock, int subclass) > > +{ > > + ___might_sleep(__FILE__, __LINE__, 0); > > This _must_ be rtlock_might_resched() like it is done in rt_write_lock() > above. I should have Cced you. Thanks for the catch. If it's fine, Andrew, could you fold it? Thank you. From 81f8721bc76d5f8c94770e53c6ad2e41aec8ab21 Mon Sep 17 00:00:00 2001 From: Minchan Kim <minchan@kernel.org> Date: Fri, 19 Nov 2021 10:15:00 -0800 Subject: [PATCH] locking/rwlocks: fix write_lock_nested for RT Fix build break of write_lock_nested for RT. Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Minchan Kim <minchan@kernel.org> --- include/linux/rwlock_rt.h | 4 ++++ kernel/locking/spinlock_rt.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h index efd6da62c893..8544ff05e594 100644 --- a/include/linux/rwlock_rt.h +++ b/include/linux/rwlock_rt.h @@ -84,10 +84,14 @@ static __always_inline void write_lock(rwlock_t *rwlock) rt_write_lock(rwlock); } +#ifdef CONFIG_DEBUG_LOCK_ALLOC static __always_inline void write_lock_nested(rwlock_t *rwlock, int subclass) { rt_write_lock_nested(rwlock, subclass); } +#else +#define write_lock_nested(lock, subclass) rt_write_lock(((void)(subclass), (lock))) +#endif static __always_inline void write_lock_bh(rwlock_t *rwlock) { diff --git a/kernel/locking/spinlock_rt.c b/kernel/locking/spinlock_rt.c index b82d346f1e00..b501aef820d5 100644 --- a/kernel/locking/spinlock_rt.c +++ b/kernel/locking/spinlock_rt.c @@ -242,7 +242,7 @@ EXPORT_SYMBOL(rt_write_lock); #ifdef CONFIG_DEBUG_LOCK_ALLOC void __sched rt_write_lock_nested(rwlock_t *rwlock, int subclass) { - ___might_sleep(__FILE__, __LINE__, 0); + rtlock_might_resched(); rwlock_acquire(&rwlock->dep_map, subclass, 0, _RET_IP_); rwbase_write_lock(&rwlock->rwbase, TASK_RTLOCK_WAIT); rcu_read_lock();
Hi Minchan,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/master]
[also build test ERROR on linux/master linus/master v5.16-rc1]
[cannot apply to hnaz-mm/master tip/locking/core next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Minchan-Kim/zsmalloc-remove-bit_spin_lock/20211116-030720
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8ab774587903771821b59471cc723bba6d893942
config: nds32-randconfig-r003-20211115 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c24db750268d85953fe12742e6e4a7b8baf16623
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Minchan-Kim/zsmalloc-remove-bit_spin_lock/20211116-030720
git checkout c24db750268d85953fe12742e6e4a7b8baf16623
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_spin_lock' [-Wmissing-prototypes]
68 | void __lockfunc __raw_##op##_lock(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:126:1: note: in expansion of macro 'BUILD_LOCK_OPS'
126 | BUILD_LOCK_OPS(spin, raw_spinlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_spin_lock_irqsave' [-Wmissing-prototypes]
80 | unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:126:1: note: in expansion of macro 'BUILD_LOCK_OPS'
126 | BUILD_LOCK_OPS(spin, raw_spinlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_spin_lock_irq' [-Wmissing-prototypes]
98 | void __lockfunc __raw_##op##_lock_irq(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:126:1: note: in expansion of macro 'BUILD_LOCK_OPS'
126 | BUILD_LOCK_OPS(spin, raw_spinlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_spin_lock_bh' [-Wmissing-prototypes]
103 | void __lockfunc __raw_##op##_lock_bh(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:126:1: note: in expansion of macro 'BUILD_LOCK_OPS'
126 | BUILD_LOCK_OPS(spin, raw_spinlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_read_lock' [-Wmissing-prototypes]
68 | void __lockfunc __raw_##op##_lock(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:129:1: note: in expansion of macro 'BUILD_LOCK_OPS'
129 | BUILD_LOCK_OPS(read, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_read_lock_irqsave' [-Wmissing-prototypes]
80 | unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:129:1: note: in expansion of macro 'BUILD_LOCK_OPS'
129 | BUILD_LOCK_OPS(read, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_read_lock_irq' [-Wmissing-prototypes]
98 | void __lockfunc __raw_##op##_lock_irq(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:129:1: note: in expansion of macro 'BUILD_LOCK_OPS'
129 | BUILD_LOCK_OPS(read, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_read_lock_bh' [-Wmissing-prototypes]
103 | void __lockfunc __raw_##op##_lock_bh(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:129:1: note: in expansion of macro 'BUILD_LOCK_OPS'
129 | BUILD_LOCK_OPS(read, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:68:17: warning: no previous prototype for '__raw_write_lock' [-Wmissing-prototypes]
68 | void __lockfunc __raw_##op##_lock(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:130:1: note: in expansion of macro 'BUILD_LOCK_OPS'
130 | BUILD_LOCK_OPS(write, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:80:26: warning: no previous prototype for '__raw_write_lock_irqsave' [-Wmissing-prototypes]
80 | unsigned long __lockfunc __raw_##op##_lock_irqsave(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:130:1: note: in expansion of macro 'BUILD_LOCK_OPS'
130 | BUILD_LOCK_OPS(write, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:98:17: warning: no previous prototype for '__raw_write_lock_irq' [-Wmissing-prototypes]
98 | void __lockfunc __raw_##op##_lock_irq(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:130:1: note: in expansion of macro 'BUILD_LOCK_OPS'
130 | BUILD_LOCK_OPS(write, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c:103:17: warning: no previous prototype for '__raw_write_lock_bh' [-Wmissing-prototypes]
103 | void __lockfunc __raw_##op##_lock_bh(locktype##_t *lock) \
| ^~~~~~
kernel/locking/spinlock.c:130:1: note: in expansion of macro 'BUILD_LOCK_OPS'
130 | BUILD_LOCK_OPS(write, rwlock);
| ^~~~~~~~~~~~~~
kernel/locking/spinlock.c: In function '_raw_write_lock_nested':
>> kernel/locking/spinlock.c:306:9: error: implicit declaration of function '__raw_write_lock_nested'; did you mean '_raw_write_lock_nested'? [-Werror=implicit-function-declaration]
306 | __raw_write_lock_nested(lock, subclass);
| ^~~~~~~~~~~~~~~~~~~~~~~
| _raw_write_lock_nested
cc1: some warnings being treated as errors
vim +306 kernel/locking/spinlock.c
303
304 void __lockfunc _raw_write_lock_nested(rwlock_t *lock, int subclass)
305 {
> 306 __raw_write_lock_nested(lock, subclass);
307 }
308 EXPORT_SYMBOL(_raw_write_lock_nested);
309 #endif
310
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 2021-11-19 10:21:37 [-0800], Minchan Kim wrote: > > #define rt_write_lock_nested(lock, subclass) rt_write_lock(((void)(subclass), (lock))) > > Guess you meant #define write_lock_nested. indeed, yes. > I should have Cced you. Thanks for the catch. > If it's fine, Andrew, could you fold it? You are welcome. I tested the series in my RT queue and it works. Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Thank you. Sebastian
diff --git a/include/linux/rwlock.h b/include/linux/rwlock.h index 2c0ad417ce3c..8f416c5e929e 100644 --- a/include/linux/rwlock.h +++ b/include/linux/rwlock.h @@ -55,6 +55,12 @@ do { \ #define write_lock(lock) _raw_write_lock(lock) #define read_lock(lock) _raw_read_lock(lock) +#ifdef CONFIG_DEBUG_LOCK_ALLOC +#define write_lock_nested(lock, subclass) _raw_write_lock_nested(lock, subclass) +#else +#define write_lock_nested(lock, subclass) _raw_write_lock(lock) +#endif + #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK) #define read_lock_irqsave(lock, flags) \ diff --git a/include/linux/rwlock_api_smp.h b/include/linux/rwlock_api_smp.h index f1db6f17c4fb..f0c535ec4e65 100644 --- a/include/linux/rwlock_api_smp.h +++ b/include/linux/rwlock_api_smp.h @@ -17,6 +17,7 @@ void __lockfunc _raw_read_lock(rwlock_t *lock) __acquires(lock); void __lockfunc _raw_write_lock(rwlock_t *lock) __acquires(lock); +void __lockfunc _raw_write_lock_nested(rwlock_t *lock, int subclass) __acquires(lock); void __lockfunc _raw_read_lock_bh(rwlock_t *lock) __acquires(lock); void __lockfunc _raw_write_lock_bh(rwlock_t *lock) __acquires(lock); void __lockfunc _raw_read_lock_irq(rwlock_t *lock) __acquires(lock); @@ -46,6 +47,7 @@ _raw_write_unlock_irqrestore(rwlock_t *lock, unsigned long flags) #ifdef CONFIG_INLINE_WRITE_LOCK #define _raw_write_lock(lock) __raw_write_lock(lock) +#define _raw_write_lock_nested(lock, subclass) __raw_write_lock_nested(lock, subclass) #endif #ifdef CONFIG_INLINE_READ_LOCK_BH @@ -209,6 +211,13 @@ static inline void __raw_write_lock(rwlock_t *lock) LOCK_CONTENDED(lock, do_raw_write_trylock, do_raw_write_lock); } +static inline void __raw_write_lock_nested(rwlock_t *lock, int subclass) +{ + preempt_disable(); + rwlock_acquire(&lock->dep_map, subclass, 0, _RET_IP_); + LOCK_CONTENDED(lock, do_raw_write_trylock, do_raw_write_lock); +} + #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */ static inline void __raw_write_unlock(rwlock_t *lock) diff --git a/include/linux/rwlock_rt.h b/include/linux/rwlock_rt.h index 49c1f3842ed5..efd6da62c893 100644 --- a/include/linux/rwlock_rt.h +++ b/include/linux/rwlock_rt.h @@ -28,6 +28,7 @@ extern void rt_read_lock(rwlock_t *rwlock); extern int rt_read_trylock(rwlock_t *rwlock); extern void rt_read_unlock(rwlock_t *rwlock); extern void rt_write_lock(rwlock_t *rwlock); +extern void rt_write_lock_nested(rwlock_t *rwlock, int subclass); extern int rt_write_trylock(rwlock_t *rwlock); extern void rt_write_unlock(rwlock_t *rwlock); @@ -83,6 +84,11 @@ static __always_inline void write_lock(rwlock_t *rwlock) rt_write_lock(rwlock); } +static __always_inline void write_lock_nested(rwlock_t *rwlock, int subclass) +{ + rt_write_lock_nested(rwlock, subclass); +} + static __always_inline void write_lock_bh(rwlock_t *rwlock) { local_bh_disable(); diff --git a/include/linux/spinlock_api_up.h b/include/linux/spinlock_api_up.h index d0d188861ad6..b8ba00ccccde 100644 --- a/include/linux/spinlock_api_up.h +++ b/include/linux/spinlock_api_up.h @@ -59,6 +59,7 @@ #define _raw_spin_lock_nested(lock, subclass) __LOCK(lock) #define _raw_read_lock(lock) __LOCK(lock) #define _raw_write_lock(lock) __LOCK(lock) +#define _raw_write_lock_nested(lock, subclass) __LOCK(lock) #define _raw_spin_lock_bh(lock) __LOCK_BH(lock) #define _raw_read_lock_bh(lock) __LOCK_BH(lock) #define _raw_write_lock_bh(lock) __LOCK_BH(lock) diff --git a/kernel/locking/spinlock.c b/kernel/locking/spinlock.c index b562f9289372..996811efa6d6 100644 --- a/kernel/locking/spinlock.c +++ b/kernel/locking/spinlock.c @@ -300,6 +300,12 @@ void __lockfunc _raw_write_lock(rwlock_t *lock) __raw_write_lock(lock); } EXPORT_SYMBOL(_raw_write_lock); + +void __lockfunc _raw_write_lock_nested(rwlock_t *lock, int subclass) +{ + __raw_write_lock_nested(lock, subclass); +} +EXPORT_SYMBOL(_raw_write_lock_nested); #endif #ifndef CONFIG_INLINE_WRITE_LOCK_IRQSAVE diff --git a/kernel/locking/spinlock_rt.c b/kernel/locking/spinlock_rt.c index b2e553f9255b..b82d346f1e00 100644 --- a/kernel/locking/spinlock_rt.c +++ b/kernel/locking/spinlock_rt.c @@ -239,6 +239,18 @@ void __sched rt_write_lock(rwlock_t *rwlock) } EXPORT_SYMBOL(rt_write_lock); +#ifdef CONFIG_DEBUG_LOCK_ALLOC +void __sched rt_write_lock_nested(rwlock_t *rwlock, int subclass) +{ + ___might_sleep(__FILE__, __LINE__, 0); + rwlock_acquire(&rwlock->dep_map, subclass, 0, _RET_IP_); + rwbase_write_lock(&rwlock->rwbase, TASK_RTLOCK_WAIT); + rcu_read_lock(); + migrate_disable(); +} +EXPORT_SYMBOL(rt_write_lock_nested); +#endif + void __sched rt_read_unlock(rwlock_t *rwlock) { rwlock_release(&rwlock->dep_map, _RET_IP_);
In preparation for converting bit_spin_lock to rwlock in zsmalloc so that multiple writers of zspages can run at the same time but those zspages are supposed to be different zspage instance. Thus, it's not deadlock. This patch adds write_lock_nested to support the case for LOCKDEP. Cc: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Minchan Kim <minchan@kernel.org> --- include/linux/rwlock.h | 6 ++++++ include/linux/rwlock_api_smp.h | 9 +++++++++ include/linux/rwlock_rt.h | 6 ++++++ include/linux/spinlock_api_up.h | 1 + kernel/locking/spinlock.c | 6 ++++++ kernel/locking/spinlock_rt.c | 12 ++++++++++++ 6 files changed, 40 insertions(+)