Re: [PATCH v2] net/irda: fix lockdep annotation
On Mon, Jan 16, 2017 at 7:09 PM, kbuild test robot 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
>273
>274 /*
>275 * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
>276 */
>277 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
>278 # include
>279 #else
>280 # include
>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 {
Re: [PATCH v2] net/irda: fix lockdep annotation
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
273
274 /*
275 * Pull the _spin_*()/_read_*()/_write_*() functions/declarations:
276 */
277 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
278 # include
279 #else
280 # include
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
[PATCH v2] net/irda: fix lockdep annotation
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
Cc: Dave Jones
Cc: Samuel Ortiz
Cc: David S. Miller
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
