2023-09-13 09:06:11

by Michał Mirosław

[permalink] [raw]
Subject: [PATCH v3] locking/mutex: remove redundant argument from __mutex_lock_common()

use_ww_ctx is equivalent to ww_ctx != NULL. The one case where
use_ww_ctx was true but ww_ctx == NULL leads to the same
__mutex_add_waiter() call via __ww_mutex_add_waiter().

Since now __ww_mutex_add_waiter() is called only with ww_mutex != NULL
(from both regular and PREEMPT_RT implementations), remove the
branch there.

Resulting object size diffs (by gcc-12) are minor:

text data bss dec hex filename (x86-64)
22603 4696 16 27315 6ab3 /tmp/before.o
22593 4696 16 27305 6aa9 /tmp/after.o

text data bss dec hex filename (arm)
13488 56 8 13552 34f0 /tmp/before.o
13492 56 8 13556 34f4 /tmp/after.o

Signed-off-by: Michał Mirosław <[email protected]>
---
v3: extended commit message with `size` diffs
+ added back `if (ww_ctx)`-guarded store: compiler hoists it into the
following branch anyway and so it avoids the unnecessary store in
the `ww_ctx == NULL` case.
v2: extended commit message to note that PREEMPT_RT does not call
__ww_mutex_add_waiter() with ww_ctx == NULL
---
kernel/locking/mutex.c | 15 ++++++---------
kernel/locking/ww_mutex.h | 5 -----
2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 4a3c006c41fb..045f7da4e473 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -578,15 +578,12 @@ EXPORT_SYMBOL(ww_mutex_unlock);
static __always_inline int __sched
__mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclass,
struct lockdep_map *nest_lock, unsigned long ip,
- struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
+ struct ww_acquire_ctx *ww_ctx)
{
struct mutex_waiter waiter;
struct ww_mutex *ww;
int ret;

- if (!use_ww_ctx)
- ww_ctx = NULL;
-
might_sleep();

MUTEX_WARN_ON(lock->magic != lock);
@@ -637,12 +634,12 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas

debug_mutex_lock_common(lock, &waiter);
waiter.task = current;
- if (use_ww_ctx)
+ if (ww_ctx)
waiter.ww_ctx = ww_ctx;

lock_contended(&lock->dep_map, ip);

- if (!use_ww_ctx) {
+ if (!ww_ctx) {
/* add waiting tasks to the end of the waitqueue (FIFO): */
__mutex_add_waiter(lock, &waiter, &lock->wait_list);
} else {
@@ -754,14 +751,14 @@ static int __sched
__mutex_lock(struct mutex *lock, unsigned int state, unsigned int subclass,
struct lockdep_map *nest_lock, unsigned long ip)
{
- return __mutex_lock_common(lock, state, subclass, nest_lock, ip, NULL, false);
+ return __mutex_lock_common(lock, state, subclass, nest_lock, ip, NULL);
}

static int __sched
__ww_mutex_lock(struct mutex *lock, unsigned int state, unsigned int subclass,
unsigned long ip, struct ww_acquire_ctx *ww_ctx)
{
- return __mutex_lock_common(lock, state, subclass, NULL, ip, ww_ctx, true);
+ return __mutex_lock_common(lock, state, subclass, NULL, ip, ww_ctx);
}

/**
@@ -841,7 +838,7 @@ mutex_lock_io_nested(struct mutex *lock, unsigned int subclass)

token = io_schedule_prepare();
__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE,
- subclass, NULL, _RET_IP_, NULL, 0);
+ subclass, NULL, _RET_IP_, NULL);
io_schedule_finish(token);
}
EXPORT_SYMBOL_GPL(mutex_lock_io_nested);
diff --git a/kernel/locking/ww_mutex.h b/kernel/locking/ww_mutex.h
index 3ad2cc4823e5..11acb2efe976 100644
--- a/kernel/locking/ww_mutex.h
+++ b/kernel/locking/ww_mutex.h
@@ -493,11 +493,6 @@ __ww_mutex_add_waiter(struct MUTEX_WAITER *waiter,
struct MUTEX_WAITER *cur, *pos = NULL;
bool is_wait_die;

- if (!ww_ctx) {
- __ww_waiter_add(lock, waiter, NULL);
- return 0;
- }
-
is_wait_die = ww_ctx->is_wait_die;

/*
--
2.39.2


2023-09-18 13:15:49

by Oliver Sang

[permalink] [raw]
Subject: Re: [PATCH v3] locking/mutex: remove redundant argument from __mutex_lock_common()



Hello,

kernel test robot noticed "canonical_address#:#[##]" on:

commit: e51feab54b4cf9e46f5f1c70a95bd783d71bea17 ("[PATCH v3] locking/mutex: remove redundant argument from __mutex_lock_common()")
url: https://github.com/intel-lab-lkp/linux/commits/Micha-Miros-aw/locking-mutex-remove-redundant-argument-from-__mutex_lock_common/20230913-040021
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 68373ebb9d61985e05574313a356f751ef9911ab
patch link: https://lore.kernel.org/all/13334f7016362b2031eb65b03cb1a49b6500957f.1694548262.git.mirq-linux@rere.qmqm.pl/
patch subject: [PATCH v3] locking/mutex: remove redundant argument from __mutex_lock_common()

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)



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 <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]


[ 32.125787][ T1] AVX2 or AES-NI instructions are not detected.
[ 32.126826][ T1] AVX or AES-NI instructions are not detected.
[ 32.127952][ T1] AVX2 or AES-NI instructions are not detected.
[ 32.131287][ T1] AVX512/GFNI instructions are not detected.
[ 32.132975][ T1] Beginning ww mutex selftests
[ 36.707495][ T37] general protection fault, probably for non-canonical address 0xfbd59c00000000a2: 0000 [#1] SMP KASAN PTI
[ 36.708950][ T37] KASAN: maybe wild-memory-access in range [0xdead000000000510-0xdead000000000517]
[ 36.708950][ T37] CPU: 0 PID: 37 Comm: kworker/u4:2 Not tainted 6.5.0-rc3-00003-ge51feab54b4c #1 d00dc367a39745ce3f654e553e9329c6c6efc292
[ 36.708950][ T37] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 36.708950][ T37] Workqueue: test-ww_mutex stress_inorder_work
[ 36.708950][ T37] RIP: 0010:__ww_mutex_check_waiters (mutex.c:?)
[ 36.708950][ T37] Code: 01 38 d0 7c 08 84 d2 0f 85 45 01 00 00 66 41 83 7f 16 00 74 45 48 b8 00 00 00 00 00 fc ff df 48 8d 7e 10 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 30 01 00 00 8b 4e 10 85 c9 75
All code
========
0: 01 38 add %edi,(%rax)
2: d0 7c 08 84 sarb -0x7c(%rax,%rcx,1)
6: d2 0f rorb %cl,(%rdi)
8: 85 45 01 test %eax,0x1(%rbp)
b: 00 00 add %al,(%rax)
d: 66 41 83 7f 16 00 cmpw $0x0,0x16(%r15)
13: 74 45 je 0x5a
15: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
1c: fc ff df
1f: 48 8d 7e 10 lea 0x10(%rsi),%rdi
23: 48 89 fa mov %rdi,%rdx
26: 48 c1 ea 03 shr $0x3,%rdx
2a:* 0f b6 04 02 movzbl (%rdx,%rax,1),%eax <-- trapping instruction
2e: 84 c0 test %al,%al
30: 74 08 je 0x3a
32: 3c 03 cmp $0x3,%al
34: 0f 8e 30 01 00 00 jle 0x16a
3a: 8b 4e 10 mov 0x10(%rsi),%ecx
3d: 85 c9 test %ecx,%ecx
3f: 75 .byte 0x75

Code starting with the faulting instruction
===========================================
0: 0f b6 04 02 movzbl (%rdx,%rax,1),%eax
4: 84 c0 test %al,%al
6: 74 08 je 0x10
8: 3c 03 cmp $0x3,%al
a: 0f 8e 30 01 00 00 jle 0x140
10: 8b 4e 10 mov 0x10(%rsi),%ecx
13: 85 c9 test %ecx,%ecx
15: 75 .byte 0x75
[ 36.708950][ T37] RSP: 0000:ffff88811273fa38 EFLAGS: 00010a02
[ 36.708950][ T37] RAX: dffffc0000000000 RBX: ffff8881108afbf0 RCX: 0000000000000000
[ 36.708950][ T37] RDX: 1bd5a000000000a2 RSI: dead000000000500 RDI: dead000000000510
[ 36.708950][ T37] RBP: ffff88815a089060 R08: 0000000000000000 R09: 0000000000000000
[ 36.708950][ T37] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88815a0890b0
[ 36.708950][ T37] R13: ffffed10224e7f90 R14: dffffc0000000000 R15: ffff88811273fc70
[ 36.708950][ T37] FS: 0000000000000000(0000) GS:ffff8883af000000(0000) knlGS:0000000000000000
[ 36.708950][ T37] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 36.708950][ T37] CR2: ffff88843ffff000 CR3: 000000000508a000 CR4: 00000000000406f0
[ 36.708950][ T37] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 36.708950][ T37] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 36.708950][ T37] Call Trace:
[ 36.708950][ T37] <TASK>
[ 36.708950][ T37] ? die_addr (??:?)
[ 36.738985][ T37] ? exc_general_protection (??:?)
[ 36.738985][ T37] ? asm_exc_general_protection (??:?)
[ 36.738985][ T37] ? __ww_mutex_check_waiters (mutex.c:?)


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230918/[email protected]



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