[PATCH v2] net/irda: fix lockdep annotation

[PATCH v2] net/irda: fix lockdep annotation

[Dmitry Vyukov]

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

Re: [PATCH v2] net/irda: fix lockdep annotation

[kbuild test robot]

[-- 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 --]

Re: [PATCH v2] net/irda: fix lockdep annotation

[Dmitry Vyukov]

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