* [PATCH v2] net/irda: fix lockdep annotation
@ 2017-01-16 14:51 Dmitry Vyukov
2017-01-16 18:09 ` kbuild test robot
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Vyukov @ 2017-01-16 14:51 UTC (permalink / raw)
To: davej, samuel, davem; +Cc: glider, andreyknvl, Dmitry Vyukov, netdev
The current annotation uses a global variable as recursion counter.
The variable is not atomic nor protected with a mutex, but mutated
by multiple threads. This causes lockdep bug reports episodically:
BUG: looking up invalid subclass: 4294967295
...
_raw_spin_lock_irqsave_nested+0x120/0x180
hashbin_delete+0x4fe/0x750
__irias_delete_object+0xab/0x170
irias_delete_object+0x5f/0xc0
ircomm_tty_detach_cable+0x1d5/0x3f0
...
Make the hashbin_lock_depth variable atomic to prevent bug reports.
As is this causes "unused variable 'depth'" warning without LOCKDEP.
So also change raw_spin_lock_irqsave_nested() macro to not cause
the warning without LOCKDEP. Similar to what raw_spin_lock_nested()
already does.
Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Dave Jones <davej@redhat.com>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Fixes: c7630a4b932af ("[IrDA]: irda lockdep annotation")
---
Changes since v1:
- Added raw_spin_lock_irqsave_nested() change
as 0-DAY bot reported compiler warning without LOCKDEP.
---
include/linux/spinlock.h | 1 +
net/irda/irqueue.c | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 47dd0cebd204..27aca8c1b129 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -217,6 +217,7 @@ static inline void do_raw_spin_unlock(raw_spinlock_t *lock) __releases(lock)
#else
#define raw_spin_lock_irqsave_nested(lock, flags, subclass) \
do { \
+ (void)subclass; \
typecheck(unsigned long, flags); \
flags = _raw_spin_lock_irqsave(lock); \
} while (0)
diff --git a/net/irda/irqueue.c b/net/irda/irqueue.c
index acbe61c7e683..b9fd74e6ca99 100644
--- a/net/irda/irqueue.c
+++ b/net/irda/irqueue.c
@@ -384,21 +384,23 @@ EXPORT_SYMBOL(hashbin_new);
* just supply kfree, which should take care of the job.
*/
#ifdef CONFIG_LOCKDEP
-static int hashbin_lock_depth = 0;
+static atomic_t hashbin_lock_depth = ATOMIC_INIT(0);
#endif
int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func)
{
irda_queue_t* queue;
unsigned long flags = 0;
- int i;
+ int i, depth = 0;
IRDA_ASSERT(hashbin != NULL, return -1;);
IRDA_ASSERT(hashbin->magic == HB_MAGIC, return -1;);
/* Synchronize */
if ( hashbin->hb_type & HB_LOCK ) {
- spin_lock_irqsave_nested(&hashbin->hb_spinlock, flags,
- hashbin_lock_depth++);
+#ifdef CONFIG_LOCKDEP
+ depth = atomic_inc_return(&hashbin_lock_depth) - 1;
+#endif
+ spin_lock_irqsave_nested(&hashbin->hb_spinlock, flags, depth);
}
/*
@@ -423,7 +425,7 @@ int hashbin_delete( hashbin_t* hashbin, FREE_FUNC free_func)
if ( hashbin->hb_type & HB_LOCK) {
spin_unlock_irqrestore(&hashbin->hb_spinlock, flags);
#ifdef CONFIG_LOCKDEP
- hashbin_lock_depth--;
+ atomic_dec(&hashbin_lock_depth);
#endif
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] net/irda: fix lockdep annotation
2017-01-16 14:51 [PATCH v2] net/irda: fix lockdep annotation Dmitry Vyukov
@ 2017-01-16 18:09 ` kbuild test robot
2017-01-16 21:11 ` Dmitry Vyukov
0 siblings, 1 reply; 3+ messages in thread
From: kbuild test robot @ 2017-01-16 18:09 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: kbuild-all, davej, samuel, davem, glider, andreyknvl,
Dmitry Vyukov, netdev
[-- Attachment #1: Type: text/plain, Size: 5629 bytes --]
Hi Dmitry,
[auto build test ERROR on tip/locking/core]
[also build test ERROR on v4.10-rc4 next-20170116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Dmitry-Vyukov/net-irda-fix-lockdep-annotation/20170117-001737
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/mmzone.h:7:0,
from include/linux/gfp.h:5,
from include/linux/slab.h:14,
from drivers/gpu/drm/i915/i915_sw_fence.c:10:
drivers/gpu/drm/i915/i915_sw_fence.c: In function '__i915_sw_fence_wake_up_all':
>> include/linux/spinlock.h:217:3: error: void value not ignored as it ought to be
(void)subclass; \
>> include/linux/spinlock.h:335:2: note: in expansion of macro 'raw_spin_lock_irqsave_nested'
raw_spin_lock_irqsave_nested(spinlock_check(lock), flags, subclass); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_sw_fence.c:68:2: note: in expansion of macro 'spin_lock_irqsave_nested'
spin_lock_irqsave_nested(&x->lock, flags, 1 + !!continuation);
^~~~~~~~~~~~~~~~~~~~~~~~
vim +217 include/linux/spinlock.h
211 typecheck(unsigned long, flags); \
212 flags = _raw_spin_lock_irqsave_nested(lock, subclass); \
213 } while (0)
214 #else
215 #define raw_spin_lock_irqsave_nested(lock, flags, subclass) \
216 do { \
> 217 (void)subclass; \
218 typecheck(unsigned long, flags); \
219 flags = _raw_spin_lock_irqsave(lock); \
220 } while (0)
221 #endif
222
223 #else
224
225 #define raw_spin_lock_irqsave(lock, flags) \
226 do { \
227 typecheck(unsigned long, flags); \
228 _raw_spin_lock_irqsave(lock, flags); \
229 } while (0)
230
231 #define raw_spin_lock_irqsave_nested(lock, flags, subclass) \
232 raw_spin_lock_irqsave(lock, flags)
233
234 #endif
235
236 #define raw_spin_lock_irq(lock) _raw_spin_lock_irq(lock)
237 #define raw_spin_lock_bh(lock) _raw_spin_lock_bh(lock)
238 #define raw_spin_unlock(lock) _raw_spin_unlock(lock)
239 #define raw_spin_unlock_irq(lock) _raw_spin_unlock_irq(lock)
240
241 #define raw_spin_unlock_irqrestore(lock, flags) \
242 do { \
243 typecheck(unsigned long, flags); \
244 _raw_spin_unlock_irqrestore(lock, flags); \
245 } while (0)
246 #define raw_spin_unlock_bh(lock) _raw_spin_unlock_bh(lock)
247
248 #define raw_spin_trylock_bh(lock) \
249 __cond_lock(lock, _raw_spin_trylock_bh(lock))
250
251 #define raw_spin_trylock_irq(lock) \
252 ({ \
253 local_irq_disable(); \
254 raw_spin_trylock(lock) ? \
255 1 : ({ local_irq_enable(); 0; }); \
256 })
257
258 #define raw_spin_trylock_irqsave(lock, flags) \
259 ({ \
260 local_irq_save(flags); \
261 raw_spin_trylock(lock) ? \
262 1 : ({ local_irq_restore(flags); 0; }); \
263 })
264
265 /**
266 * raw_spin_can_lock - would raw_spin_trylock() succeed?
267 * @lock: the spinlock in question.
268 */
269 #define raw_spin_can_lock(lock) (!raw_spin_is_locked(lock))
270
271 /* Include rwlock functions */
272 #include <linux/rwlock.h>
273
274 /*
275 * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
276 */
277 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
278 # include <linux/spinlock_api_smp.h>
279 #else
280 # include <linux/spinlock_api_up.h>
281 #endif
282
283 /*
284 * Map the spin_lock functions to the raw variants for PREEMPT_RT=n
285 */
286
287 static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock)
288 {
289 return &lock->rlock;
290 }
291
292 #define spin_lock_init(_lock) \
293 do { \
294 spinlock_check(_lock); \
295 raw_spin_lock_init(&(_lock)->rlock); \
296 } while (0)
297
298 static __always_inline void spin_lock(spinlock_t *lock)
299 {
300 raw_spin_lock(&lock->rlock);
301 }
302
303 static __always_inline void spin_lock_bh(spinlock_t *lock)
304 {
305 raw_spin_lock_bh(&lock->rlock);
306 }
307
308 static __always_inline int spin_trylock(spinlock_t *lock)
309 {
310 return raw_spin_trylock(&lock->rlock);
311 }
312
313 #define spin_lock_nested(lock, subclass) \
314 do { \
315 raw_spin_lock_nested(spinlock_check(lock), subclass); \
316 } while (0)
317
318 #define spin_lock_nest_lock(lock, nest_lock) \
319 do { \
320 raw_spin_lock_nest_lock(spinlock_check(lock), nest_lock); \
321 } while (0)
322
323 static __always_inline void spin_lock_irq(spinlock_t *lock)
324 {
325 raw_spin_lock_irq(&lock->rlock);
326 }
327
328 #define spin_lock_irqsave(lock, flags) \
329 do { \
330 raw_spin_lock_irqsave(spinlock_check(lock), flags); \
331 } while (0)
332
333 #define spin_lock_irqsave_nested(lock, flags, subclass) \
334 do { \
> 335 raw_spin_lock_irqsave_nested(spinlock_check(lock), flags, subclass); \
336 } while (0)
337
338 static __always_inline void spin_unlock(spinlock_t *lock)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25439 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] net/irda: fix lockdep annotation
2017-01-16 18:09 ` kbuild test robot
@ 2017-01-16 21:11 ` Dmitry Vyukov
0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Vyukov @ 2017-01-16 21:11 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Dave Jones, Samuel Ortiz, David Miller,
Alexander Potapenko, andreyknvl, netdev
On Mon, Jan 16, 2017 at 7:09 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Dmitry,
>
> [auto build test ERROR on tip/locking/core]
> [also build test ERROR on v4.10-rc4 next-20170116]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Dmitry-Vyukov/net-irda-fix-lockdep-annotation/20170117-001737
> config: i386-defconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/mmzone.h:7:0,
> from include/linux/gfp.h:5,
> from include/linux/slab.h:14,
> from drivers/gpu/drm/i915/i915_sw_fence.c:10:
> drivers/gpu/drm/i915/i915_sw_fence.c: In function '__i915_sw_fence_wake_up_all':
>>> include/linux/spinlock.h:217:3: error: void value not ignored as it ought to be
> (void)subclass; \
>
>>> include/linux/spinlock.h:335:2: note: in expansion of macro 'raw_spin_lock_irqsave_nested'
> raw_spin_lock_irqsave_nested(spinlock_check(lock), flags, subclass); \
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/gpu/drm/i915/i915_sw_fence.c:68:2: note: in expansion of macro 'spin_lock_irqsave_nested'
> spin_lock_irqsave_nested(&x->lock, flags, 1 + !!continuation);
> ^~~~~~~~~~~~~~~~~~~~~~~~
Mailed v3.
> vim +217 include/linux/spinlock.h
>
> 211 typecheck(unsigned long, flags); \
> 212 flags = _raw_spin_lock_irqsave_nested(lock, subclass); \
> 213 } while (0)
> 214 #else
> 215 #define raw_spin_lock_irqsave_nested(lock, flags, subclass) \
> 216 do { \
> > 217 (void)subclass; \
> 218 typecheck(unsigned long, flags); \
> 219 flags = _raw_spin_lock_irqsave(lock); \
> 220 } while (0)
> 221 #endif
> 222
> 223 #else
> 224
> 225 #define raw_spin_lock_irqsave(lock, flags) \
> 226 do { \
> 227 typecheck(unsigned long, flags); \
> 228 _raw_spin_lock_irqsave(lock, flags); \
> 229 } while (0)
> 230
> 231 #define raw_spin_lock_irqsave_nested(lock, flags, subclass) \
> 232 raw_spin_lock_irqsave(lock, flags)
> 233
> 234 #endif
> 235
> 236 #define raw_spin_lock_irq(lock) _raw_spin_lock_irq(lock)
> 237 #define raw_spin_lock_bh(lock) _raw_spin_lock_bh(lock)
> 238 #define raw_spin_unlock(lock) _raw_spin_unlock(lock)
> 239 #define raw_spin_unlock_irq(lock) _raw_spin_unlock_irq(lock)
> 240
> 241 #define raw_spin_unlock_irqrestore(lock, flags) \
> 242 do { \
> 243 typecheck(unsigned long, flags); \
> 244 _raw_spin_unlock_irqrestore(lock, flags); \
> 245 } while (0)
> 246 #define raw_spin_unlock_bh(lock) _raw_spin_unlock_bh(lock)
> 247
> 248 #define raw_spin_trylock_bh(lock) \
> 249 __cond_lock(lock, _raw_spin_trylock_bh(lock))
> 250
> 251 #define raw_spin_trylock_irq(lock) \
> 252 ({ \
> 253 local_irq_disable(); \
> 254 raw_spin_trylock(lock) ? \
> 255 1 : ({ local_irq_enable(); 0; }); \
> 256 })
> 257
> 258 #define raw_spin_trylock_irqsave(lock, flags) \
> 259 ({ \
> 260 local_irq_save(flags); \
> 261 raw_spin_trylock(lock) ? \
> 262 1 : ({ local_irq_restore(flags); 0; }); \
> 263 })
> 264
> 265 /**
> 266 * raw_spin_can_lock - would raw_spin_trylock() succeed?
> 267 * @lock: the spinlock in question.
> 268 */
> 269 #define raw_spin_can_lock(lock) (!raw_spin_is_locked(lock))
> 270
> 271 /* Include rwlock functions */
> 272 #include <linux/rwlock.h>
> 273
> 274 /*
> 275 * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
> 276 */
> 277 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> 278 # include <linux/spinlock_api_smp.h>
> 279 #else
> 280 # include <linux/spinlock_api_up.h>
> 281 #endif
> 282
> 283 /*
> 284 * Map the spin_lock functions to the raw variants for PREEMPT_RT=n
> 285 */
> 286
> 287 static __always_inline raw_spinlock_t *spinlock_check(spinlock_t *lock)
> 288 {
> 289 return &lock->rlock;
> 290 }
> 291
> 292 #define spin_lock_init(_lock) \
> 293 do { \
> 294 spinlock_check(_lock); \
> 295 raw_spin_lock_init(&(_lock)->rlock); \
> 296 } while (0)
> 297
> 298 static __always_inline void spin_lock(spinlock_t *lock)
> 299 {
> 300 raw_spin_lock(&lock->rlock);
> 301 }
> 302
> 303 static __always_inline void spin_lock_bh(spinlock_t *lock)
> 304 {
> 305 raw_spin_lock_bh(&lock->rlock);
> 306 }
> 307
> 308 static __always_inline int spin_trylock(spinlock_t *lock)
> 309 {
> 310 return raw_spin_trylock(&lock->rlock);
> 311 }
> 312
> 313 #define spin_lock_nested(lock, subclass) \
> 314 do { \
> 315 raw_spin_lock_nested(spinlock_check(lock), subclass); \
> 316 } while (0)
> 317
> 318 #define spin_lock_nest_lock(lock, nest_lock) \
> 319 do { \
> 320 raw_spin_lock_nest_lock(spinlock_check(lock), nest_lock); \
> 321 } while (0)
> 322
> 323 static __always_inline void spin_lock_irq(spinlock_t *lock)
> 324 {
> 325 raw_spin_lock_irq(&lock->rlock);
> 326 }
> 327
> 328 #define spin_lock_irqsave(lock, flags) \
> 329 do { \
> 330 raw_spin_lock_irqsave(spinlock_check(lock), flags); \
> 331 } while (0)
> 332
> 333 #define spin_lock_irqsave_nested(lock, flags, subclass) \
> 334 do { \
> > 335 raw_spin_lock_irqsave_nested(spinlock_check(lock), flags, subclass); \
> 336 } while (0)
> 337
> 338 static __always_inline void spin_unlock(spinlock_t *lock)
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-16 21:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 14:51 [PATCH v2] net/irda: fix lockdep annotation Dmitry Vyukov
2017-01-16 18:09 ` kbuild test robot
2017-01-16 21:11 ` Dmitry Vyukov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.