Re: [PATCH-tip 5/5] locking/locktorture: locking/locktorture: Fix incorrect use of ww_acquire_ctx in ww_mutex test

Re: [PATCH-tip 5/5] locking/locktorture: locking/locktorture: Fix incorrect use of ww_acquire_ctx in ww_mutex test

[kernel test robot]

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

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210318172814.4400-6-longman@redhat.com>
References: <20210318172814.4400-6-longman@redhat.com>
TO: Waiman Long <longman@redhat.com>
TO: Peter Zijlstra <peterz@infradead.org>
TO: Ingo Molnar <mingo@redhat.com>
TO: Will Deacon <will@kernel.org>
TO: Boqun Feng <boqun.feng@gmail.com>
TO: "Paul E. McKenney" <paulmck@kernel.org>
TO: Davidlohr Bueso <dave@stgolabs.net>
CC: linux-kernel(a)vger.kernel.org
CC: Juri Lelli <juri.lelli@redhat.com>
CC: Waiman Long <longman@redhat.com>

Hi Waiman,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/master]
[also build test WARNING on next-20210318]
[cannot apply to rcu/dev tip/locking/core linus/master v5.12-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Waiman-Long/locking-locktorture-Fix-locktorture-ww_mutex-test-problems/20210319-013130
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 3cfcc2de9320a826032ab2ffb1cce730963533d7
:::::: branch date: 89 minutes ago
:::::: commit date: 89 minutes ago
config: arc-randconfig-s032-20210318 (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-277-gc089cd2d-dirty
        # https://github.com/0day-ci/linux/commit/c6f355ecdfac3d2f547bbd586d56d94591a06f21
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Waiman-Long/locking-locktorture-Fix-locktorture-ww_mutex-test-problems/20210319-013130
        git checkout c6f355ecdfac3d2f547bbd586d56d94591a06f21
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   kernel/locking/locktorture.c:331:12: sparse: sparse: context imbalance in 'torture_mutex_lock' - wrong count at exit
   kernel/locking/locktorture.c:352:13: sparse: sparse: context imbalance in 'torture_mutex_unlock' - wrong count at exit
   kernel/locking/locktorture.c:397:12: sparse: sparse: context imbalance in 'torture_ww_mutex_lock' - wrong count at exit
   kernel/locking/locktorture.c: note: in included file (through include/linux/seqlock.h, include/linux/mmzone.h, include/linux/gfp.h, ...):
>> include/linux/ww_mutex.h:167:9: sparse: sparse: context imbalance in 'torture_ww_mutex_unlock' - wrong count at exit
   kernel/locking/locktorture.c:470:12: sparse: sparse: context imbalance in 'torture_rtmutex_lock' - wrong count at exit
   kernel/locking/locktorture.c:527:13: sparse: sparse: context imbalance in 'torture_rtmutex_unlock' - wrong count at exit
   kernel/locking/locktorture.c:546:12: sparse: sparse: context imbalance in 'torture_rwsem_down_write' - wrong count at exit
   kernel/locking/locktorture.c:567:13: sparse: sparse: context imbalance in 'torture_rwsem_up_write' - wrong count at exit
   kernel/locking/locktorture.c:573:12: sparse: sparse: context imbalance in 'torture_rwsem_down_read' - wrong count at exit
   kernel/locking/locktorture.c:594:13: sparse: sparse: context imbalance in 'torture_rwsem_up_read' - wrong count at exit
   kernel/locking/locktorture.c:624:12: sparse: sparse: context imbalance in 'torture_percpu_rwsem_down_write' - wrong count at exit
   kernel/locking/locktorture.c:631:13: sparse: sparse: context imbalance in 'torture_percpu_rwsem_up_write' - wrong count at exit
   kernel/locking/locktorture.c: note: in included file:
   include/linux/percpu-rwsem.h:70:9: sparse: sparse: context imbalance in 'torture_percpu_rwsem_down_read' - wrong count at exit
   include/linux/percpu-rwsem.h:121:9: sparse: sparse: context imbalance in 'torture_percpu_rwsem_up_read' - wrong count at exit

vim +/torture_ww_mutex_unlock +167 include/linux/ww_mutex.h

1b375dc3071018 Maarten Lankhorst 2013-07-05  153  
1b375dc3071018 Maarten Lankhorst 2013-07-05  154  /**
1b375dc3071018 Maarten Lankhorst 2013-07-05  155   * ww_acquire_fini - releases a w/w acquire context
1b375dc3071018 Maarten Lankhorst 2013-07-05  156   * @ctx: the acquire context to free
1b375dc3071018 Maarten Lankhorst 2013-07-05  157   *
1b375dc3071018 Maarten Lankhorst 2013-07-05  158   * Releases a w/w acquire context. This must be called _after_ all acquired w/w
1b375dc3071018 Maarten Lankhorst 2013-07-05  159   * mutexes have been released with ww_mutex_unlock.
1b375dc3071018 Maarten Lankhorst 2013-07-05  160   */
1b375dc3071018 Maarten Lankhorst 2013-07-05  161  static inline void ww_acquire_fini(struct ww_acquire_ctx *ctx)
1b375dc3071018 Maarten Lankhorst 2013-07-05  162  {
bee645788e07ee Waiman Long       2021-03-16  163  #ifdef CONFIG_DEBUG_LOCK_ALLOC
5facae4f3549b5 Qian Cai          2019-09-19  164  	mutex_release(&ctx->dep_map, _THIS_IP_);
bee645788e07ee Waiman Long       2021-03-16  165  #endif
bee645788e07ee Waiman Long       2021-03-16  166  #ifdef CONFIG_DEBUG_MUTEXES
1b375dc3071018 Maarten Lankhorst 2013-07-05 @167  	DEBUG_LOCKS_WARN_ON(ctx->acquired);
97f2645f358b41 Masahiro Yamada   2016-08-03  168  	if (!IS_ENABLED(CONFIG_PROVE_LOCKING))
1b375dc3071018 Maarten Lankhorst 2013-07-05  169  		/*
1b375dc3071018 Maarten Lankhorst 2013-07-05  170  		 * lockdep will normally handle this,
1b375dc3071018 Maarten Lankhorst 2013-07-05  171  		 * but fail without anyway
1b375dc3071018 Maarten Lankhorst 2013-07-05  172  		 */
1b375dc3071018 Maarten Lankhorst 2013-07-05  173  		ctx->done_acquire = 1;
1b375dc3071018 Maarten Lankhorst 2013-07-05  174  
97f2645f358b41 Masahiro Yamada   2016-08-03  175  	if (!IS_ENABLED(CONFIG_DEBUG_LOCK_ALLOC))
1b375dc3071018 Maarten Lankhorst 2013-07-05  176  		/* ensure ww_acquire_fini will still fail if called twice */
1b375dc3071018 Maarten Lankhorst 2013-07-05  177  		ctx->acquired = ~0U;
1b375dc3071018 Maarten Lankhorst 2013-07-05  178  #endif
1b375dc3071018 Maarten Lankhorst 2013-07-05  179  }
1b375dc3071018 Maarten Lankhorst 2013-07-05  180  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29267 bytes --]

[PATCH-tip 5/5] locking/locktorture: locking/locktorture: Fix incorrect use of ww_acquire_ctx in ww_mutex test

[Waiman Long]

The ww_acquire_ctx structure for ww_mutex needs to persist for a complete
lock/unlock cycle. In the ww_mutex test in locktorture, however, both
ww_acquire_init() and ww_acquire_fini() are called within the lock
function only. This causes a lockdep splat of "WARNING: Nested lock
was not taken" when lockdep is enabled in the kernel.

To fix this problem, we need to move the ww_acquire_fini() after
the ww_mutex_unlock() in torture_ww_mutex_unlock(). This is done by
allocating a global array of ww_acquire_ctx structures. Each locking
thread is associated with its own ww_acquire_ctx via the unique thread
id it has so that both the lock and unlock functions can access the
same ww_acquire_ctx structure.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/locking/locktorture.c | 39 +++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 90a975a95a13..b3adb40549bf 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -374,15 +374,27 @@ static struct lock_torture_ops mutex_lock_ops = {
  */
 static DEFINE_WD_CLASS(torture_ww_class);
 static struct ww_mutex torture_ww_mutex_0, torture_ww_mutex_1, torture_ww_mutex_2;
+static struct ww_acquire_ctx *ww_acquire_ctxs;
 
 static void torture_ww_mutex_init(void)
 {
 	ww_mutex_init(&torture_ww_mutex_0, &torture_ww_class);
 	ww_mutex_init(&torture_ww_mutex_1, &torture_ww_class);
 	ww_mutex_init(&torture_ww_mutex_2, &torture_ww_class);
+
+	ww_acquire_ctxs = kmalloc_array(cxt.nrealwriters_stress,
+					sizeof(*ww_acquire_ctxs),
+					GFP_KERNEL);
+	if (!ww_acquire_ctxs)
+		VERBOSE_TOROUT_STRING("ww_acquire_ctx: Out of memory");
+}
+
+static void torture_ww_mutex_exit(void)
+{
+	kfree(ww_acquire_ctxs);
 }
 
-static int torture_ww_mutex_lock(int tid __maybe_unused)
+static int torture_ww_mutex_lock(int tid)
 __acquires(torture_ww_mutex_0)
 __acquires(torture_ww_mutex_1)
 __acquires(torture_ww_mutex_2)
@@ -392,7 +404,7 @@ __acquires(torture_ww_mutex_2)
 		struct list_head link;
 		struct ww_mutex *lock;
 	} locks[3], *ll, *ln;
-	struct ww_acquire_ctx ctx;
+	struct ww_acquire_ctx *ctx = &ww_acquire_ctxs[tid];
 
 	locks[0].lock = &torture_ww_mutex_0;
 	list_add(&locks[0].link, &list);
@@ -403,12 +415,12 @@ __acquires(torture_ww_mutex_2)
 	locks[2].lock = &torture_ww_mutex_2;
 	list_add(&locks[2].link, &list);
 
-	ww_acquire_init(&ctx, &torture_ww_class);
+	ww_acquire_init(ctx, &torture_ww_class);
 
 	list_for_each_entry(ll, &list, link) {
 		int err;
 
-		err = ww_mutex_lock(ll->lock, &ctx);
+		err = ww_mutex_lock(ll->lock, ctx);
 		if (!err)
 			continue;
 
@@ -419,26 +431,29 @@ __acquires(torture_ww_mutex_2)
 		if (err != -EDEADLK)
 			return err;
 
-		ww_mutex_lock_slow(ll->lock, &ctx);
+		ww_mutex_lock_slow(ll->lock, ctx);
 		list_move(&ll->link, &list);
 	}
 
-	ww_acquire_fini(&ctx);
 	return 0;
 }
 
-static void torture_ww_mutex_unlock(int tid __maybe_unused)
+static void torture_ww_mutex_unlock(int tid)
 __releases(torture_ww_mutex_0)
 __releases(torture_ww_mutex_1)
 __releases(torture_ww_mutex_2)
 {
+	struct ww_acquire_ctx *ctx = &ww_acquire_ctxs[tid];
+
 	ww_mutex_unlock(&torture_ww_mutex_0);
 	ww_mutex_unlock(&torture_ww_mutex_1);
 	ww_mutex_unlock(&torture_ww_mutex_2);
+	ww_acquire_fini(ctx);
 }
 
 static struct lock_torture_ops ww_mutex_lock_ops = {
 	.init		= torture_ww_mutex_init,
+	.exit		= torture_ww_mutex_exit,
 	.writelock	= torture_ww_mutex_lock,
 	.write_delay	= torture_mutex_delay,
 	.task_boost     = torture_boost_dummy,
@@ -924,16 +939,16 @@ static int __init lock_torture_init(void)
 		goto unwind;
 	}
 
-	if (cxt.cur_ops->init) {
-		cxt.cur_ops->init();
-		cxt.init_called = true;
-	}
-
 	if (nwriters_stress >= 0)
 		cxt.nrealwriters_stress = nwriters_stress;
 	else
 		cxt.nrealwriters_stress = 2 * num_online_cpus();
 
+	if (cxt.cur_ops->init) {
+		cxt.cur_ops->init();
+		cxt.init_called = true;
+	}
+
 #ifdef CONFIG_DEBUG_MUTEXES
 	if (str_has_prefix(torture_type, "mutex"))
 		cxt.debug_lock = true;
-- 
2.18.1