Re: Problem: lockdep warning with nested instances of i2c-mux

Re: Problem: lockdep warning with nested instances of i2c-mux

[Peter Rosin]

On 2018-05-24 04:25, John Sperbeck wrote:
> If an i2c topology has instances of nested muxes, then a lockdep splat
> is produced when when i2c_parent_lock_bus() is called.  Here is an
> example:
> 
> ============================================
> WARNING: possible recursive locking detected
> --------------------------------------------
> insmod/68159 is trying to acquire lock:
>   (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
> 
> but task is already holding lock:
>   (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
> 
> other info that might help us debug this:
>   Possible unsafe locking scenario:
> 
>         CPU0
>         ----
>    lock(i2c_register_adapter#2);
>    lock(i2c_register_adapter#2);
> 
>   *** DEADLOCK ***
> 
>   May be due to missing lock nesting notation
> 
> 1 lock held by insmod/68159:
>   #0:  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50
> [i2c_mux]
> 
> stack backtrace:
> CPU: 13 PID: 68159 Comm: insmod Tainted: G           O
> Call Trace:
>   dump_stack+0x67/0x98
>   __lock_acquire+0x162e/0x1780
>   lock_acquire+0xba/0x200
>   rt_mutex_lock+0x44/0x60
>   i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
>   i2c_parent_lock_bus+0x3e/0x50 [i2c_mux]
>   i2c_smbus_xfer+0xf0/0x700
>   i2c_smbus_read_byte+0x42/0x70
>   my2c_init+0xa2/0x1000 [my2c]
>   do_one_initcall+0x51/0x192
>   do_init_module+0x62/0x216
>   load_module+0x20f9/0x2b50
>   SYSC_init_module+0x19a/0x1c0
>   SyS_init_module+0xe/0x10
>   do_syscall_64+0x6c/0x1a0
>   entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> 
> The warning makes sense from the lockdep detector's point-of-view because
> we are locking two instances of a single lock class.  Normally, this would
> be addressed by using 'nested' variants of locks.  But rt_mutex doesn't
> expose an API for that, and it's not clear how i2c-mux can know what level
> of nesting it's at anyway.

Yes, when I modified the i2c-mux locking a couple of years ago, I also
noted the absense, and even tried to implement it, but eventually gave
up. However, that was before lockdep could even track rt_mutexes. Now
it looks easy, and I will follow up with a couple of patches (only
compile-tested, please test).

> In short, I don't have an easy patch to suggest.  But I'm not very
> familiar with the i2c code, and maybe I'm overlooking something that
> would help?
> 
> I have code for a module that emulates a chain of an i2c adapter, two
> muxes, and a slave device to show the problem.  On my system, with a
> kernel compiled with lockdep enabled, loading the module produces the
> splat.  I can post it, if the issue isn't clear from my description.

Not needed, the issue is known, I just wasn't aware that lockdep had
grown knowledge of rt-mutexes.

Thanks for the report!

Cheers,
Peter

Peter Rosin (2):
  rtmutex: allow specifying a subclass for nested locking
  i2c: mux: annotate the nested rt_mutex usage

 drivers/i2c/i2c-core-base.c |  2 +-
 drivers/i2c/i2c-mux.c       |  4 ++--
 include/linux/rtmutex.h     |  6 ++++++
 kernel/locking/rtmutex.c    | 29 +++++++++++++++++++++++++----
 4 files changed, 34 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH 1/2] rtmutex: allow specifying a subclass for nested locking

[Peter Rosin]

Needed for annotating rt_mutex locks.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 include/linux/rtmutex.h  |  6 ++++++
 kernel/locking/rtmutex.c | 29 +++++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 1b92a28dd672..32e18527be64 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -106,7 +106,13 @@ static inline int rt_mutex_is_locked(struct rt_mutex *lock)
 extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct lock_class_key *key);
 extern void rt_mutex_destroy(struct rt_mutex *lock);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass);
+#else
 extern void rt_mutex_lock(struct rt_mutex *lock);
+# define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock)
+#endif
+
 extern int rt_mutex_lock_interruptible(struct rt_mutex *lock);
 extern int rt_mutex_timed_lock(struct rt_mutex *lock,
 			       struct hrtimer_sleeper *timeout);
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 4f014be7a4b8..d33bc45b9d64 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1465,6 +1465,29 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
 		rt_mutex_postunlock(&wake_q);
 }
 
+static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
+{
+	might_sleep();
+
+	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
+	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+}
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+/**
+ * rt_mutex_lock_nested - lock a rt_mutex
+ *
+ * @lock: the rt_mutex to be locked
+ * @subclass: the lockdep subclass
+ */
+void __sched rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass)
+{
+	__rt_mutex_lock(lock, subclass);
+}
+EXPORT_SYMBOL_GPL(mutex_lock_nested);
+#endif
+
+#ifndef CONFIG_DEBUG_LOCK_ALLOC
 /**
  * rt_mutex_lock - lock a rt_mutex
  *
@@ -1472,12 +1495,10 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
  */
 void __sched rt_mutex_lock(struct rt_mutex *lock)
 {
-	might_sleep();
-
-	mutex_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+	__rt_mutex_lock(lock, 0);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_lock);
+#endif
 
 /**
  * rt_mutex_lock_interruptible - lock a rt_mutex interruptible
-- 
2.11.0

Re: [PATCH 1/2] rtmutex: allow specifying a subclass for nested locking

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

Hi Peter,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc6]
[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/Peter-Rosin/rtmutex-allow-specifying-a-subclass-for-nested-locking/20180526-140421
config: x86_64-randconfig-i0-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   kernel/locking/locktorture.c: In function 'torture_rtmutex_lock':
>> kernel/locking/locktorture.c:444:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'ww_mutex_lock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&torture_rtmutex);
     ^~~~~~~~~~~~~
     ww_mutex_lock
   cc1: some warnings being treated as errors

vim +444 kernel/locking/locktorture.c

095777c4 Davidlohr Bueso 2015-07-22  441  
095777c4 Davidlohr Bueso 2015-07-22  442  static int torture_rtmutex_lock(void) __acquires(torture_rtmutex)
095777c4 Davidlohr Bueso 2015-07-22  443  {
095777c4 Davidlohr Bueso 2015-07-22 @444  	rt_mutex_lock(&torture_rtmutex);
095777c4 Davidlohr Bueso 2015-07-22  445  	return 0;
095777c4 Davidlohr Bueso 2015-07-22  446  }
095777c4 Davidlohr Bueso 2015-07-22  447  

:::::: The code at line 444 was first introduced by commit
:::::: 095777c417db142970adeb776fa0cb10810b8122 locktorture: Support rtmutex torturing

:::::: TO: Davidlohr Bueso <dave@stgolabs.net>
:::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

---
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: 26734 bytes --]

Re: [PATCH 1/2] rtmutex: allow specifying a subclass for nested locking

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

Hi Peter,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc6]
[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/Peter-Rosin/rtmutex-allow-specifying-a-subclass-for-nested-locking/20180526-140421
config: x86_64-randconfig-x003-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Peter-Rosin/rtmutex-allow-specifying-a-subclass-for-nested-locking/20180526-140421 HEAD e9f3abe10927b5d6e565ac45d0814e6198b49649 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/i2c/i2c-core-base.c: In function 'i2c_adapter_lock_bus':
>> drivers/i2c/i2c-core-base.c:618:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&adapter->bus_lock);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors
--
   drivers/i2c/i2c-mux.c: In function 'i2c_mux_lock_bus':
>> drivers/i2c/i2c-mux.c:147:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&parent->mux_lock);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors

vim +618 drivers/i2c/i2c-core-base.c

3b5f794b drivers/i2c/i2c-core.c Jean Delvare   2010-06-03  608  
9c1600ed drivers/i2c/i2c-core.c David Brownell 2007-05-01  609  /**
8320f495 drivers/i2c/i2c-core.c Peter Rosin    2016-05-04  610   * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11  611   * @adapter: Target I2C bus segment
8320f495 drivers/i2c/i2c-core.c Peter Rosin    2016-05-04  612   * @flags: I2C_LOCK_ROOT_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT
8320f495 drivers/i2c/i2c-core.c Peter Rosin    2016-05-04  613   *	locks only this branch in the adapter tree
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11  614   */
8320f495 drivers/i2c/i2c-core.c Peter Rosin    2016-05-04  615  static void i2c_adapter_lock_bus(struct i2c_adapter *adapter,
8320f495 drivers/i2c/i2c-core.c Peter Rosin    2016-05-04  616  				 unsigned int flags)
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11  617  {
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11 @618  	rt_mutex_lock(&adapter->bus_lock);
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11  619  }
fe61e07e drivers/i2c/i2c-core.c Jean Delvare   2010-08-11  620  

:::::: The code at line 618 was first introduced by commit
:::::: fe61e07e9ebc890c70d97a1f72ddaad4bee2d848 i2c: Move adapter locking helpers to i2c-core

:::::: TO: Jean Delvare <khali@linux-fr.org>
:::::: CC: Jean Delvare <khali@linux-fr.org>

---
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: 27488 bytes --]

Re: [PATCH 1/2] rtmutex: allow specifying a subclass for nested locking

[kbuild test robot]

Hi Peter,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc6]
[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/Peter-Rosin/rtmutex-allow-specifying-a-subclass-for-nested-locking/20180526-140421
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   kernel/locking/locktorture.c:444:9: sparse: undefined identifier 'rt_mutex_lock'
   kernel/locking/locktorture.c:586:6: sparse: symbol 'torture_percpu_rwsem_init' was not declared. Should it be static?
   kernel/locking/locktorture.c:331:12: sparse: context imbalance in 'torture_mutex_lock' - wrong count at exit
   kernel/locking/locktorture.c:351:13: sparse: context imbalance in 'torture_mutex_unlock' - wrong count at exit
   kernel/locking/locktorture.c:373:12: sparse: context imbalance in 'torture_ww_mutex_lock' - wrong count at exit
   kernel/locking/locktorture.c:418:13: sparse: context imbalance in 'torture_ww_mutex_unlock' - wrong count at exit
>> kernel/locking/locktorture.c:444:22: sparse: call with no type!
   kernel/locking/locktorture.c:442:12: sparse: context imbalance in 'torture_rtmutex_lock' - wrong count at exit
   kernel/locking/locktorture.c:504:13: sparse: context imbalance in 'torture_rtmutex_unlock' - wrong count at exit
   kernel/locking/locktorture.c:522:12: sparse: context imbalance in 'torture_rwsem_down_write' - wrong count at exit
   kernel/locking/locktorture.c:542:13: sparse: context imbalance in 'torture_rwsem_up_write' - wrong count at exit
   kernel/locking/locktorture.c:547:12: sparse: context imbalance in 'torture_rwsem_down_read' - wrong count at exit
   kernel/locking/locktorture.c:567:13: sparse: context imbalance in 'torture_rwsem_up_read' - wrong count at exit
   kernel/locking/locktorture.c:591:12: sparse: context imbalance in 'torture_percpu_rwsem_down_write' - wrong count at exit
   kernel/locking/locktorture.c:597:13: sparse: context imbalance in 'torture_percpu_rwsem_up_write' - wrong count at exit
   include/linux/percpu-rwsem.h:50:9: sparse: context imbalance in 'torture_percpu_rwsem_down_read' - wrong count at exit
   include/linux/percpu-rwsem.h:100:9: sparse: context imbalance in 'torture_percpu_rwsem_up_read' - wrong count at exit
   kernel/locking/locktorture.c: In function 'torture_rtmutex_lock':
   kernel/locking/locktorture.c:444:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'ww_mutex_lock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&torture_rtmutex);
     ^~~~~~~~~~~~~
     ww_mutex_lock
   cc1: some warnings being treated as errors
--
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:4225:9: sparse: undefined identifier 'rt_mutex_lock'
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:4225:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c: In function 'atomisp_css_wait_acc_finish':
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:4225:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&isp->mutex);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors
--
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:774:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:910:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:1174:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:1265:9: sparse: undefined identifier 'rt_mutex_lock'
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:774:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:910:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:1174:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:1265:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c: In function 'atomisp_open':
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c:774:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&isp->mutex);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors
--
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:663:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:680:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:781:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:829:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:842:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:857:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:870:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:888:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1109:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1171:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1302:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1377:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1439:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1460:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1696:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1757:33: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2036:17: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2182:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2213:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2292:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2466:25: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2577:25: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2593:25: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2655:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2677:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2735:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2775:17: sparse: undefined identifier 'rt_mutex_lock'
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:663:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:680:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:781:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:829:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:842:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:857:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:870:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:888:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1109:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1171:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1302:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1377:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1439:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1460:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1696:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:1757:46: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2036:30: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2182:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2213:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2292:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2466:38: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2577:38: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2593:38: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2655:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2677:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2735:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:2775:30: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c: In function 'atomisp_g_input':
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c:663:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&isp->mutex);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors
--
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1466:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1891:9: sparse: undefined identifier 'rt_mutex_lock'
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:3302:43: sparse: incorrect type in argument 2 (different address spaces) @@    expected void const [noderef] <asn:1>*from @@    got ef] <asn:1>*from @@
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:3302:43:    expected void const [noderef] <asn:1>*from
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:3302:43:    got void const *from
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4070:58: sparse: incorrect type in argument 2 (different address spaces) @@    expected void const *from @@    got unsigned short [nodervoid const *from @@
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4070:58:    expected void const *from
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4070:58:    got unsigned short [noderef] <asn:1>*<noident>
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4082:58: sparse: incorrect type in argument 2 (different address spaces) @@    expected void const *from @@    got unsigned short [nodervoid const *from @@
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4082:58:    expected void const *from
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4082:58:    got unsigned short [noderef] <asn:1>*<noident>
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4827:35: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4827:35: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4986:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4989:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5020:28: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5023:29: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5875:36: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5879:37: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5967:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5967:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5970:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5970:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6179:62: sparse: incorrect type in argument 2 (different address spaces) @@    expected void const [noderef] <asn:1>*from @@    got id const [noderef] <asn:1>*from @@
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6179:62:    expected void const [noderef] <asn:1>*from
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6179:62:    got unsigned short [usertype] *<noident>
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6327:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6327:33: sparse: expression using sizeof(void)
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:6332:33: sparse: expression using sizeof(void)
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1466:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1891:22: sparse: call with no type!
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c: In function 'atomisp_wdt_work':
   drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1466:2: error: implicit declaration of function 'rt_mutex_lock'; did you mean 'rt_mutex_unlock'? [-Werror=implicit-function-declaration]
     rt_mutex_lock(&isp->mutex);
     ^~~~~~~~~~~~~
     rt_mutex_unlock
   cc1: some warnings being treated as errors

vim +444 kernel/locking/locktorture.c

095777c4 Davidlohr Bueso 2015-07-22  441  
095777c4 Davidlohr Bueso 2015-07-22  442  static int torture_rtmutex_lock(void) __acquires(torture_rtmutex)
095777c4 Davidlohr Bueso 2015-07-22  443  {
095777c4 Davidlohr Bueso 2015-07-22 @444  	rt_mutex_lock(&torture_rtmutex);
095777c4 Davidlohr Bueso 2015-07-22  445  	return 0;
095777c4 Davidlohr Bueso 2015-07-22  446  }
095777c4 Davidlohr Bueso 2015-07-22  447  

:::::: The code at line 444 was first introduced by commit
:::::: 095777c417db142970adeb776fa0cb10810b8122 locktorture: Support rtmutex torturing

:::::: TO: Davidlohr Bueso <dave@stgolabs.net>
:::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH 2/2] i2c: mux: annotate the nested rt_mutex usage

[Peter Rosin]

If an i2c topology has instances of nested muxes, then a lockdep splat
is produced when when i2c_parent_lock_bus() is called.  Here is an
example:

============================================
WARNING: possible recursive locking detected
--------------------------------------------
insmod/68159 is trying to acquire lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

but task is already holding lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(i2c_register_adapter#2);
   lock(i2c_register_adapter#2);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

1 lock held by insmod/68159:
  #0:  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50
[i2c_mux]

stack backtrace:
CPU: 13 PID: 68159 Comm: insmod Tainted: G           O
Call Trace:
  dump_stack+0x67/0x98
  __lock_acquire+0x162e/0x1780
  lock_acquire+0xba/0x200
  rt_mutex_lock+0x44/0x60
  i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
  i2c_parent_lock_bus+0x3e/0x50 [i2c_mux]
  i2c_smbus_xfer+0xf0/0x700
  i2c_smbus_read_byte+0x42/0x70
  my2c_init+0xa2/0x1000 [my2c]
  do_one_initcall+0x51/0x192
  do_init_module+0x62/0x216
  load_module+0x20f9/0x2b50
  SYSC_init_module+0x19a/0x1c0
  SyS_init_module+0xe/0x10
  do_syscall_64+0x6c/0x1a0
  entry_SYSCALL_64_after_hwframe+0x42/0xb7

Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-core-base.c | 2 +-
 drivers/i2c/i2c-mux.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index f5ec6ec6776f..1157a64c7be3 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -615,7 +615,7 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 static void i2c_adapter_lock_bus(struct i2c_adapter *adapter,
 				 unsigned int flags)
 {
-	rt_mutex_lock(&adapter->bus_lock);
+	rt_mutex_lock_nested(&adapter->bus_lock, i2c_adapter_depth(adapter));
 }
 
 /**
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 9669ca4937b8..7ba31f6bf148 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -144,7 +144,7 @@ static void i2c_mux_lock_bus(struct i2c_adapter *adapter, unsigned int flags)
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	if (!(flags & I2C_LOCK_ROOT_ADAPTER))
 		return;
 	i2c_lock_bus(parent, flags);
@@ -181,7 +181,7 @@ static void i2c_parent_lock_bus(struct i2c_adapter *adapter,
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	i2c_lock_bus(parent, flags);
 }
 
-- 
2.11.0

Re: [PATCH 2/2] i2c: mux: annotate the nested rt_mutex usage

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Hi Peter,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc6]
[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/Peter-Rosin/rtmutex-allow-specifying-a-subclass-for-nested-locking/20180526-140421
config: i386-randconfig-s0-201820 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   WARNING: vmlinux: 'mutex_lock_nested' exported twice. Previous export was in vmlinux
>> ERROR: "rt_mutex_lock_nested" [drivers/i2c/i2c-mux.ko] undefined!
>> ERROR: "rt_mutex_lock_nested" [drivers/i2c/i2c-core.ko] undefined!

---
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: 25256 bytes --]

[PATCH v2 0/2] Re: Problem: lockdep warning with nested instances of i2c-mux

[Peter Rosin]

Changes since v1:
- Further compile tests indicated a missing #define for rt_mutex_lock
  with lockdep enabled, so that one is added.
- I have verified that I don't get any lockdep splat for a local i2c-mux
  setup with these patches applied, and that I do without them.

Again, thanks for the report!

Cheers,
Peter

Peter Rosin (2):
  rtmutex: allow specifying a subclass for nested locking
  i2c: mux: annotate the nested rt_mutex usage

 drivers/i2c/i2c-core-base.c |  2 +-
 drivers/i2c/i2c-mux.c       |  4 ++--
 include/linux/rtmutex.h     |  7 +++++++
 kernel/locking/rtmutex.c    | 29 +++++++++++++++++++++++++----
 4 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH v2 1/2] rtmutex: allow specifying a subclass for nested locking

[Peter Rosin]

Needed for annotating rt_mutex locks.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 include/linux/rtmutex.h  |  7 +++++++
 kernel/locking/rtmutex.c | 29 +++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 1b92a28dd672..6fd615a0eea9 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -106,7 +106,14 @@ static inline int rt_mutex_is_locked(struct rt_mutex *lock)
 extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct lock_class_key *key);
 extern void rt_mutex_destroy(struct rt_mutex *lock);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass);
+#define rt_mutex_lock(lock) rt_mutex_lock_nested(lock, 0)
+#else
 extern void rt_mutex_lock(struct rt_mutex *lock);
+#define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock)
+#endif
+
 extern int rt_mutex_lock_interruptible(struct rt_mutex *lock);
 extern int rt_mutex_timed_lock(struct rt_mutex *lock,
 			       struct hrtimer_sleeper *timeout);
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 4f014be7a4b8..d33bc45b9d64 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1465,6 +1465,29 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
 		rt_mutex_postunlock(&wake_q);
 }
 
+static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
+{
+	might_sleep();
+
+	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
+	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+}
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+/**
+ * rt_mutex_lock_nested - lock a rt_mutex
+ *
+ * @lock: the rt_mutex to be locked
+ * @subclass: the lockdep subclass
+ */
+void __sched rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass)
+{
+	__rt_mutex_lock(lock, subclass);
+}
+EXPORT_SYMBOL_GPL(mutex_lock_nested);
+#endif
+
+#ifndef CONFIG_DEBUG_LOCK_ALLOC
 /**
  * rt_mutex_lock - lock a rt_mutex
  *
@@ -1472,12 +1495,10 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
  */
 void __sched rt_mutex_lock(struct rt_mutex *lock)
 {
-	might_sleep();
-
-	mutex_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+	__rt_mutex_lock(lock, 0);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_lock);
+#endif
 
 /**
  * rt_mutex_lock_interruptible - lock a rt_mutex interruptible
-- 
2.11.0

[PATCH v2 2/2] i2c: mux: annotate the nested rt_mutex usage

[Peter Rosin]

If an i2c topology has instances of nested muxes, then a lockdep splat
is produced when when i2c_parent_lock_bus() is called.  Here is an
example:

============================================
WARNING: possible recursive locking detected
--------------------------------------------
insmod/68159 is trying to acquire lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

but task is already holding lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(i2c_register_adapter#2);
   lock(i2c_register_adapter#2);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

1 lock held by insmod/68159:
  #0:  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50
[i2c_mux]

stack backtrace:
CPU: 13 PID: 68159 Comm: insmod Tainted: G           O
Call Trace:
  dump_stack+0x67/0x98
  __lock_acquire+0x162e/0x1780
  lock_acquire+0xba/0x200
  rt_mutex_lock+0x44/0x60
  i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
  i2c_parent_lock_bus+0x3e/0x50 [i2c_mux]
  i2c_smbus_xfer+0xf0/0x700
  i2c_smbus_read_byte+0x42/0x70
  my2c_init+0xa2/0x1000 [my2c]
  do_one_initcall+0x51/0x192
  do_init_module+0x62/0x216
  load_module+0x20f9/0x2b50
  SYSC_init_module+0x19a/0x1c0
  SyS_init_module+0xe/0x10
  do_syscall_64+0x6c/0x1a0
  entry_SYSCALL_64_after_hwframe+0x42/0xb7

Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-core-base.c | 2 +-
 drivers/i2c/i2c-mux.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index f5ec6ec6776f..1157a64c7be3 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -615,7 +615,7 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 static void i2c_adapter_lock_bus(struct i2c_adapter *adapter,
 				 unsigned int flags)
 {
-	rt_mutex_lock(&adapter->bus_lock);
+	rt_mutex_lock_nested(&adapter->bus_lock, i2c_adapter_depth(adapter));
 }
 
 /**
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 9669ca4937b8..7ba31f6bf148 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -144,7 +144,7 @@ static void i2c_mux_lock_bus(struct i2c_adapter *adapter, unsigned int flags)
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	if (!(flags & I2C_LOCK_ROOT_ADAPTER))
 		return;
 	i2c_lock_bus(parent, flags);
@@ -181,7 +181,7 @@ static void i2c_parent_lock_bus(struct i2c_adapter *adapter,
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	i2c_lock_bus(parent, flags);
 }
 
-- 
2.11.0

[PATCH v3 0/2] Re: Problem: lockdep warning with nested instances of i2c-mux

[Peter Rosin]

Hi!

Sorry for spamming. At least I'm finding these embarrassing f$&%ups
myself, not that it helps all that much, but...

Changes since v2    https://lkml.org/lkml/2018/5/24/176
- EXPORT_SYMBOL_GPL(rt_mutex_lock_nested) is more appropriate (the
  rt_ prefix was missing).

Changes since v1    https://lkml.org/lkml/2018/5/24/93
- Further compile tests indicated a missing #define for rt_mutex_lock
  with lockdep enabled, so that one is added.
- I have verified that I don't get any lockdep splat for a local i2c-mux
  setup with these patches applied, and that I do without them.

Cheers,
Peter

Peter Rosin (2):
  rtmutex: allow specifying a subclass for nested locking
  i2c: mux: annotate the nested rt_mutex usage

 drivers/i2c/i2c-core-base.c |  2 +-
 drivers/i2c/i2c-mux.c       |  4 ++--
 include/linux/rtmutex.h     |  7 +++++++
 kernel/locking/rtmutex.c    | 29 +++++++++++++++++++++++++----
 4 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking

[Peter Rosin]

Needed for annotating rt_mutex locks.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 include/linux/rtmutex.h  |  7 +++++++
 kernel/locking/rtmutex.c | 29 +++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 1b92a28dd672..6fd615a0eea9 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -106,7 +106,14 @@ static inline int rt_mutex_is_locked(struct rt_mutex *lock)
 extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct lock_class_key *key);
 extern void rt_mutex_destroy(struct rt_mutex *lock);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass);
+#define rt_mutex_lock(lock) rt_mutex_lock_nested(lock, 0)
+#else
 extern void rt_mutex_lock(struct rt_mutex *lock);
+#define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock)
+#endif
+
 extern int rt_mutex_lock_interruptible(struct rt_mutex *lock);
 extern int rt_mutex_timed_lock(struct rt_mutex *lock,
 			       struct hrtimer_sleeper *timeout);
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 4f014be7a4b8..2823d4163a37 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1465,6 +1465,29 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
 		rt_mutex_postunlock(&wake_q);
 }
 
+static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
+{
+	might_sleep();
+
+	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
+	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+}
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+/**
+ * rt_mutex_lock_nested - lock a rt_mutex
+ *
+ * @lock: the rt_mutex to be locked
+ * @subclass: the lockdep subclass
+ */
+void __sched rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass)
+{
+	__rt_mutex_lock(lock, subclass);
+}
+EXPORT_SYMBOL_GPL(rt_mutex_lock_nested);
+#endif
+
+#ifndef CONFIG_DEBUG_LOCK_ALLOC
 /**
  * rt_mutex_lock - lock a rt_mutex
  *
@@ -1472,12 +1495,10 @@ rt_mutex_fastunlock(struct rt_mutex *lock,
  */
 void __sched rt_mutex_lock(struct rt_mutex *lock)
 {
-	might_sleep();
-
-	mutex_acquire(&lock->dep_map, 0, 0, _RET_IP_);
-	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
+	__rt_mutex_lock(lock, 0);
 }
 EXPORT_SYMBOL_GPL(rt_mutex_lock);
+#endif
 
 /**
  * rt_mutex_lock_interruptible - lock a rt_mutex interruptible
-- 
2.11.0

Re: [PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking

[Joel Fernandes]

On Thu, May 24, 2018 at 03:52:39PM +0200, Peter Rosin wrote:
> Needed for annotating rt_mutex locks.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  include/linux/rtmutex.h  |  7 +++++++
>  kernel/locking/rtmutex.c | 29 +++++++++++++++++++++++++----
>  2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
> index 1b92a28dd672..6fd615a0eea9 100644
> --- a/include/linux/rtmutex.h
> +++ b/include/linux/rtmutex.h
> @@ -106,7 +106,14 @@ static inline int rt_mutex_is_locked(struct rt_mutex *lock)
>  extern void __rt_mutex_init(struct rt_mutex *lock, const char *name, struct lock_class_key *key);
>  extern void rt_mutex_destroy(struct rt_mutex *lock);
>  
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +extern void rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass);
> +#define rt_mutex_lock(lock) rt_mutex_lock_nested(lock, 0)
> +#else
>  extern void rt_mutex_lock(struct rt_mutex *lock);
> +#define rt_mutex_lock_nested(lock, subclass) rt_mutex_lock(lock)
> +#endif
> +
>  extern int rt_mutex_lock_interruptible(struct rt_mutex *lock);
>  extern int rt_mutex_timed_lock(struct rt_mutex *lock,
>  			       struct hrtimer_sleeper *timeout);
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
:
>  }
>  
> +static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
> +{
> +	might_sleep();
> +
> +	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
> +	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
> +}
> +
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +/**
> + * rt_mutex_lock_nested - lock a rt_mutex

This ifdef seems consistent with other nested locking primitives, but its
kind of confusing.

The Kconfig.debug for DEBUG_LOCK_ALLOC says:

config DEBUG_LOCK_ALLOC
	bool "Lock debugging: detect incorrect freeing of live locks"
	[...]
	help
	 This feature will check whether any held lock (spinlock, rwlock,
	 mutex or rwsem) is incorrectly freed by the kernel, via any of the
	 memory-freeing routines (kfree(), kmem_cache_free(), free_pages(),
	 vfree(), etc.), whether a live lock is incorrectly reinitialized via
	 spin_lock_init()/mutex_init()/etc., or whether there is any lock
	 held during task exit.

Shouldn't this ideally be ifdef'd under PROVE_LOCKING for this and other
locking primitives? Any idea what's the reason? I know PROVE_LOCKING selects
DEBUG_LOCK_ALLOC but still..

thanks!

- Joel
 

Re: [PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking

[Peter Zijlstra]

On Sun, May 27, 2018 at 10:19:36PM -0700, Joel Fernandes wrote:

> > +static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
> > +{
> > +	might_sleep();
> > +
> > +	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
> > +	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
> > +}
> > +
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > +/**
> > + * rt_mutex_lock_nested - lock a rt_mutex
> 
> This ifdef seems consistent with other nested locking primitives, but its
> kind of confusing.
> 
> The Kconfig.debug for DEBUG_LOCK_ALLOC says:
> 
> config DEBUG_LOCK_ALLOC
> 	bool "Lock debugging: detect incorrect freeing of live locks"
> 	[...]
> 	help
> 	 This feature will check whether any held lock (spinlock, rwlock,
> 	 mutex or rwsem) is incorrectly freed by the kernel, via any of the
> 	 memory-freeing routines (kfree(), kmem_cache_free(), free_pages(),
> 	 vfree(), etc.), whether a live lock is incorrectly reinitialized via
> 	 spin_lock_init()/mutex_init()/etc., or whether there is any lock
> 	 held during task exit.
> 
> Shouldn't this ideally be ifdef'd under PROVE_LOCKING for this and other
> locking primitives? Any idea what's the reason? I know PROVE_LOCKING selects
> DEBUG_LOCK_ALLOC but still..

No, the reason is that DEBUG_LOCK_ALLOC needs the lockdep hooks to know
which locks are held, so it can warn when we try and free a held one.
PROVE_LOCKING builds upon that.

The the locking primitives should key off of DEBUG_LOCK_ALLOC for
introducing the hooks.

Re: [PATCH v3 1/2] rtmutex: allow specifying a subclass for nested locking

[Joel Fernandes]

On Mon, May 28, 2018 at 09:17:51AM +0200, Peter Zijlstra wrote:
> On Sun, May 27, 2018 at 10:19:36PM -0700, Joel Fernandes wrote:
> 
> > > +static inline void __rt_mutex_lock(struct rt_mutex *lock, unsigned int subclass)
> > > +{
> > > +	might_sleep();
> > > +
> > > +	mutex_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
> > > +	rt_mutex_fastlock(lock, TASK_UNINTERRUPTIBLE, rt_mutex_slowlock);
> > > +}
> > > +
> > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > +/**
> > > + * rt_mutex_lock_nested - lock a rt_mutex
> > 
> > This ifdef seems consistent with other nested locking primitives, but its
> > kind of confusing.
> > 
> > The Kconfig.debug for DEBUG_LOCK_ALLOC says:
> > 
> > config DEBUG_LOCK_ALLOC
> > 	bool "Lock debugging: detect incorrect freeing of live locks"
> > 	[...]
> > 	help
> > 	 This feature will check whether any held lock (spinlock, rwlock,
> > 	 mutex or rwsem) is incorrectly freed by the kernel, via any of the
> > 	 memory-freeing routines (kfree(), kmem_cache_free(), free_pages(),
> > 	 vfree(), etc.), whether a live lock is incorrectly reinitialized via
> > 	 spin_lock_init()/mutex_init()/etc., or whether there is any lock
> > 	 held during task exit.
> > 
> > Shouldn't this ideally be ifdef'd under PROVE_LOCKING for this and other
> > locking primitives? Any idea what's the reason? I know PROVE_LOCKING selects
> > DEBUG_LOCK_ALLOC but still..
> 
> No, the reason is that DEBUG_LOCK_ALLOC needs the lockdep hooks to know
> which locks are held, so it can warn when we try and free a held one.
> PROVE_LOCKING builds upon that.
> 
> The the locking primitives should key off of DEBUG_LOCK_ALLOC for
> introducing the hooks.

Got it, thanks for the clarification Peter!

Regards,

 -Joel

[PATCH v3 2/2] i2c: mux: annotate the nested rt_mutex usage

[Peter Rosin]

If an i2c topology has instances of nested muxes, then a lockdep splat
is produced when when i2c_parent_lock_bus() is called.  Here is an
example:

============================================
WARNING: possible recursive locking detected
--------------------------------------------
insmod/68159 is trying to acquire lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

but task is already holding lock:
  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50 [i2c_mux]

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(i2c_register_adapter#2);
   lock(i2c_register_adapter#2);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

1 lock held by insmod/68159:
  #0:  (i2c_register_adapter#2){+.+.}, at: i2c_parent_lock_bus+0x32/0x50
[i2c_mux]

stack backtrace:
CPU: 13 PID: 68159 Comm: insmod Tainted: G           O
Call Trace:
  dump_stack+0x67/0x98
  __lock_acquire+0x162e/0x1780
  lock_acquire+0xba/0x200
  rt_mutex_lock+0x44/0x60
  i2c_parent_lock_bus+0x32/0x50 [i2c_mux]
  i2c_parent_lock_bus+0x3e/0x50 [i2c_mux]
  i2c_smbus_xfer+0xf0/0x700
  i2c_smbus_read_byte+0x42/0x70
  my2c_init+0xa2/0x1000 [my2c]
  do_one_initcall+0x51/0x192
  do_init_module+0x62/0x216
  load_module+0x20f9/0x2b50
  SYSC_init_module+0x19a/0x1c0
  SyS_init_module+0xe/0x10
  do_syscall_64+0x6c/0x1a0
  entry_SYSCALL_64_after_hwframe+0x42/0xb7

Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-core-base.c | 2 +-
 drivers/i2c/i2c-mux.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index f5ec6ec6776f..1157a64c7be3 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -615,7 +615,7 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 static void i2c_adapter_lock_bus(struct i2c_adapter *adapter,
 				 unsigned int flags)
 {
-	rt_mutex_lock(&adapter->bus_lock);
+	rt_mutex_lock_nested(&adapter->bus_lock, i2c_adapter_depth(adapter));
 }
 
 /**
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 9669ca4937b8..7ba31f6bf148 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -144,7 +144,7 @@ static void i2c_mux_lock_bus(struct i2c_adapter *adapter, unsigned int flags)
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	if (!(flags & I2C_LOCK_ROOT_ADAPTER))
 		return;
 	i2c_lock_bus(parent, flags);
@@ -181,7 +181,7 @@ static void i2c_parent_lock_bus(struct i2c_adapter *adapter,
 	struct i2c_mux_priv *priv = adapter->algo_data;
 	struct i2c_adapter *parent = priv->muxc->parent;
 
-	rt_mutex_lock(&parent->mux_lock);
+	rt_mutex_lock_nested(&parent->mux_lock, i2c_adapter_depth(adapter));
 	i2c_lock_bus(parent, flags);
 }
 
-- 
2.11.0

Re: [PATCH v3 0/2] Re: Problem: lockdep warning with nested instances of i2c-mux

[John Sperbeck]

On Thu, May 24, 2018 at 6:52 AM Peter Rosin <peda@axentia.se> wrote:

> Hi!

> Sorry for spamming. At least I'm finding these embarrassing f$&%ups
> myself, not that it helps all that much, but...

> Changes since v2    https://lkml.org/lkml/2018/5/24/176
> - EXPORT_SYMBOL_GPL(rt_mutex_lock_nested) is more appropriate (the
>    rt_ prefix was missing).


Yes, after fixing the "rt_" typo, this addresses our use case.  Thanks for
the quick response.