[PATCH] Support lockref reference count if enable LOCK_STAT

[PATCH] Support lockref reference count if enable LOCK_STAT

[yongli-oc]

Swap the positions of lock and count to support CMPXCHG_LOCKREF
if SPINLOCK_SIZE > 4 when enable LOCK_STAT. The reference count
can always be used regardless of the spinlock_t size.

Signed-off-by: yongli-oc <yongli-oc@zhaoxin.com>
---
 include/linux/lockref.h | 17 ++++++++++++++++-
 lib/lockref.c           | 26 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/linux/lockref.h b/include/linux/lockref.h
index c3a1f78bc884..44ac754a029b 100644
--- a/include/linux/lockref.h
+++ b/include/linux/lockref.h
@@ -22,15 +22,30 @@
 	(IS_ENABLED(CONFIG_ARCH_USE_CMPXCHG_LOCKREF) && \
 	 IS_ENABLED(CONFIG_SMP) && SPINLOCK_SIZE <= 4)
 
+#define USE_CMPXCHG_LOCKREF_ALTERNATIVE \
+	(IS_ENABLED(CONFIG_ARCH_USE_CMPXCHG_LOCKREF) && \
+	 IS_ENABLED(CONFIG_SMP) && SPINLOCK_SIZE > 4)
+
 struct lockref {
 	union {
 #if USE_CMPXCHG_LOCKREF
 		aligned_u64 lock_count;
-#endif
 		struct {
 			spinlock_t lock;
 			int count;
 		};
+#elif USE_CMPXCHG_LOCKREF_ALTERNATIVE
+		aligned_u64 lock_count;
+		struct {
+			int count;
+			spinlock_t lock;
+		} __packed;
+#else
+		struct {
+			spinlock_t lock;
+			int count;
+		};
+#endif
 	};
 };
 
diff --git a/lib/lockref.c b/lib/lockref.c
index 2afe4c5d8919..e92606f66e9b 100644
--- a/lib/lockref.c
+++ b/lib/lockref.c
@@ -26,6 +26,32 @@
 	}									\
 } while (0)
 
+#elif USE_CMPXCHG_LOCKREF_ALTERNATIVE
+
+/*
+ * Note that the "cmpxchg()" reloads the "old" value for the
+ * failure case.
+ */
+#define CMPXCHG_LOOP(CODE, SUCCESS) do {					\
+	int retry = 100;							\
+	struct lockref old;							\
+	BUILD_BUG_ON(offsetof(struct lockref, lock) != 4);			\
+	BUILD_BUG_ON(offsetof(spinlock_t, rlock) != 0);				\
+	BUILD_BUG_ON(offsetof(raw_spinlock_t, raw_lock) != 0);			\
+	old.lock_count = READ_ONCE(lockref->lock_count);			\
+	while (likely(arch_spin_value_unlocked(old.lock.rlock.raw_lock))) {	\
+		struct lockref new = old;					\
+		CODE								\
+		if (likely(try_cmpxchg64_relaxed(&lockref->lock_count,		\
+						 &old.lock_count,		\
+						 new.lock_count))) {		\
+			SUCCESS;						\
+		}								\
+		if (!--retry)							\
+			break;							\
+	}									\
+} while (0)
+
 #else
 
 #define CMPXCHG_LOOP(CODE, SUCCESS) do { } while (0)
-- 
2.34.1

Re: [PATCH] Support lockref reference count if enable LOCK_STAT

[kernel test robot]

Hi yongli-oc,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on akpm-mm/mm-nonmm-unstable linus/master v6.12-rc7 next-20241113]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/yongli-oc/Support-lockref-reference-count-if-enable-LOCK_STAT/20241113-171348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20241113085703.148839-1-yongli-oc%40zhaoxin.com
patch subject: [PATCH] Support lockref reference count if enable LOCK_STAT
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20241114/202411140553.K9M15p3F-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241114/202411140553.K9M15p3F-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411140553.K9M15p3F-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/fs.h:8,
                    from include/linux/huge_mm.h:7,
                    from include/linux/mm.h:1120,
                    from include/linux/pid_namespace.h:7,
                    from include/linux/ptrace.h:10,
                    from include/linux/elfcore.h:11,
                    from include/linux/vmcore_info.h:6,
                    from include/linux/kexec.h:18,
                    from arch/s390/purgatory/purgatory.c:10:
   include/linux/dcache.h: In function 'dont_mount':
>> include/linux/dcache.h:369:19: warning: taking address of packed member of 'struct <anonymous>' may result in an unaligned pointer value [-Waddress-of-packed-member]
     369 |         spin_lock(&dentry->d_lock);
         |                   ^
   include/linux/dcache.h:371:21: warning: taking address of packed member of 'struct <anonymous>' may result in an unaligned pointer value [-Waddress-of-packed-member]
     371 |         spin_unlock(&dentry->d_lock);
         |                     ^


vim +369 include/linux/dcache.h

d83c49f3e36cec Al Viro 2010-04-30  366  
d83c49f3e36cec Al Viro 2010-04-30  367  static inline void dont_mount(struct dentry *dentry)
d83c49f3e36cec Al Viro 2010-04-30  368  {
d83c49f3e36cec Al Viro 2010-04-30 @369  	spin_lock(&dentry->d_lock);
d83c49f3e36cec Al Viro 2010-04-30  370  	dentry->d_flags |= DCACHE_CANT_MOUNT;
d83c49f3e36cec Al Viro 2010-04-30  371  	spin_unlock(&dentry->d_lock);
d83c49f3e36cec Al Viro 2010-04-30  372  }
d83c49f3e36cec Al Viro 2010-04-30  373  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] Support lockref reference count if enable LOCK_STAT

[kernel test robot]

Hi yongli-oc,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on akpm-mm/mm-nonmm-unstable linus/master v6.12-rc7 next-20241113]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/yongli-oc/Support-lockref-reference-count-if-enable-LOCK_STAT/20241113-171348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20241113085703.148839-1-yongli-oc%40zhaoxin.com
patch subject: [PATCH] Support lockref reference count if enable LOCK_STAT
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241114/202411140548.NBssFC0E-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241114/202411140548.NBssFC0E-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411140548.NBssFC0E-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/s390/purgatory/purgatory.c:10:
   In file included from include/linux/kexec.h:18:
   In file included from include/linux/vmcore_info.h:6:
   In file included from include/linux/elfcore.h:11:
   In file included from include/linux/ptrace.h:10:
   In file included from include/linux/pid_namespace.h:7:
   In file included from include/linux/mm.h:1120:
   In file included from include/linux/huge_mm.h:7:
   In file included from include/linux/fs.h:8:
>> include/linux/dcache.h:369:13: warning: taking address of packed member 'lock' of class or structure 'lockref::(anonymous union)::struct (anonymous at include/linux/lockref.h:39:3)' may result in an unaligned pointer value [-Waddress-of-packed-member]
     369 |         spin_lock(&dentry->d_lock);
         |                    ^~~~~~~~~~~~~~
   include/linux/dcache.h:371:15: warning: taking address of packed member 'lock' of class or structure 'lockref::(anonymous union)::struct (anonymous at include/linux/lockref.h:39:3)' may result in an unaligned pointer value [-Waddress-of-packed-member]
     371 |         spin_unlock(&dentry->d_lock);
         |                      ^~~~~~~~~~~~~~
   2 warnings generated.


vim +369 include/linux/dcache.h

d83c49f3e36cec Al Viro 2010-04-30  366  
d83c49f3e36cec Al Viro 2010-04-30  367  static inline void dont_mount(struct dentry *dentry)
d83c49f3e36cec Al Viro 2010-04-30  368  {
d83c49f3e36cec Al Viro 2010-04-30 @369  	spin_lock(&dentry->d_lock);
d83c49f3e36cec Al Viro 2010-04-30  370  	dentry->d_flags |= DCACHE_CANT_MOUNT;
d83c49f3e36cec Al Viro 2010-04-30  371  	spin_unlock(&dentry->d_lock);
d83c49f3e36cec Al Viro 2010-04-30  372  }
d83c49f3e36cec Al Viro 2010-04-30  373  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] Support lockref reference count if enable LOCK_STAT

[Andrew Morton]

On Wed, 13 Nov 2024 16:57:03 +0800 yongli-oc <yongli-oc@zhaoxin.com> wrote:

> Swap the positions of lock and count to support CMPXCHG_LOCKREF
> if SPINLOCK_SIZE > 4 when enable LOCK_STAT. The reference count
> can always be used regardless of the spinlock_t size.

Please fully describe the problem we're solving here?  And what are the
runtime effects of this change?

Re: [PATCH] Support lockref reference count if enable LOCK_STAT

[yongli-oc]

Hi, Morton:
One of the struct lockref is used in dcache.h. The spinlock_t is 4
bytes, with the 4 bytes reference count, the struct lockref is a 8
bytes variable, the count can be inc/dec by a 64 bits atomic operation,
when the spinlock is unlocked.
If the spinlock is more than 4 bytes, such as enable the kernel config
DEBUG_SPINLOCK or LOCK_STAT, the count inc/dec should got the spinlock
first, the sequence likes "lock; inc_ref; unlock;", which will caused
spinlock contention increased so much.
The chart below shows the spinlock contention with and without the
lockref patch. it got by "cat /proc/lock_stat" after ubuntu 22.04 boot
up. I tried each case two times. the con-bounces and contentions
of 6.6.28 with LOCK_STAT enable are much more than it of the 6.6.28
lockref patched. 
With the lockref patch, the count can be operated by atomic too. The
statistics result is similar to the 6.6.28 kernel production
environment.

lock_stat version 0.4
  --------------------------------------------------
  bootup  class name     con-bounces    contentions
  times
  --------kernel 6.6.28, enable LOCK_STAT-----------
   1   &dentry->d_lock:    385336         413098
   2   &dentry->d_lock:    378268         402761
  --------------------------------------------------
  -------6.6.28 with lockref patch and LOCK_STAT----
   1   &dentry->d_lock:     79941          82431
   2   &dentry->d_lock:     77817          80301
  --------------------------------------------------
Li Yong

On Thu, 2024-11-14 at 16:14 -0800, Andrew Morton wrote:
> 
> 
> [这封邮件来自外部发件人 谨防风险]
> 
> On Wed, 13 Nov 2024 16:57:03 +0800 yongli-oc <yongli-oc@zhaoxin.com>
> wrote:
> 
> > Swap the positions of lock and count to support CMPXCHG_LOCKREF
> > if SPINLOCK_SIZE > 4 when enable LOCK_STAT. The reference count
> > can always be used regardless of the spinlock_t size.
> 
> Please fully describe the problem we're solving here?  And what are
> the
> runtime effects of this change?