* [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. @ 2013-09-07 16:00 Tetsuo Handa 2013-09-08 4:32 ` Tetsuo Handa 0 siblings, 1 reply; 15+ messages in thread From: Tetsuo Handa @ 2013-09-07 16:00 UTC (permalink / raw) To: maarten.lankhorst Cc: linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Hello. I noticed that 3.11 and current linux.git do not boot (they hang before printing the "Linux version 3.10.0-rc7-00026-g040a0a3" line) when built with CONFIG_DEBUG_MUTEXES=y using gcc (GCC) 3.3.5 (Debian 1:3.3.5-13). They boot OK when built with the same config using gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3. Bisection reached commit 040a0a37 "mutex: Add support for wound/wait style locks". This commit might contain gcc version dependent trick, but how can I find it? Kernel config (only for testing whether the kernel version line is printed) is at http://I-love.SAKURA.ne.jp/tmp/config-3.11-mutex and the command line I used for testing is $ qemu-system-i386 -m 512 -nographic -kernel arch/x86/boot/bzImage --append "console=ttyS0,115200n8" . Regards. ---------- bisection log start ---------- # bad: [ad81f0545ef01ea651886dddac4bef6cec930092] Linux 3.11-rc1 # good: [8bb495e3f02401ee6f76d1b1d77f3ac9f079e376] Linux 3.10 # good: [c1be5a5b1b355d40e6cf79cc979eb66dafa24ad1] Linux 3.9 # good: [19f949f52599ba7c3f67a5897ac6be14bfcb1200] Linux 3.8 # good: [29594404d7fe73cd80eaa4ee8c43dcc53970c60e] Linux 3.7 # good: [a0d271cbfed1dd50278c6b06bead3d00ba0a88f9] Linux 3.6 # good: [28a33cbc24e4256c143dce96c7d93bf423229f92] Linux 3.5 # good: [76e10d158efb6d4516018846f60c2ab5501900bc] Linux 3.4 # good: [c16fa4f2ad19908a47c63d8fa436a1178438c7e7] Linux 3.3 # good: [805a6af8dba5dfdd35ec35dc52ec0122400b2610] Linux 3.2 # good: [c3b92c8787367a8bb53d57d9789b558f1295cc96] Linux 3.1 # good: [02f8c6aee8df3cdc935e9bdd4f2d020306035dbe] Linux 3.0 git bisect start 'v3.11-rc1' 'v3.10' 'v3.9' 'v3.8' 'v3.7' 'v3.6' 'v3.5' 'v3.4' 'v3.3' 'v3.2' 'v3.1' 'v3.0' # bad: [1286da8bc009cb2aee7f285e94623fc974c0c983] Merge tag 'sound-3.11' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound git bisect bad 1286da8bc009cb2aee7f285e94623fc974c0c983 # good: [ee1a8d402e7e204d57fb108aa40003b6d1633036] Merge tag 'dt-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc git bisect good ee1a8d402e7e204d57fb108aa40003b6d1633036 # bad: [3e34131a65127e73fbae68c82748f32c8af7e4a4] Merge tag 'stable/for-linus-3.11-rc0-tag-two' of git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen git bisect bad 3e34131a65127e73fbae68c82748f32c8af7e4a4 # bad: [790eac5640abf7a57fa3a644386df330e18c11b0] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs git bisect bad 790eac5640abf7a57fa3a644386df330e18c11b0 # bad: [f0bb4c0ab064a8aeeffbda1cee380151a594eaab] Merge branch 'perf-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad f0bb4c0ab064a8aeeffbda1cee380151a594eaab # good: [3e42dee676e8cf5adca817b1518b2e99d1c138ff] Merge branch 'core-locking-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect good 3e42dee676e8cf5adca817b1518b2e99d1c138ff # good: [130768b8c93cd8d21390a136ec8cef417153ca14] perf/x86/intel: Add Haswell PEBS record support git bisect good 130768b8c93cd8d21390a136ec8cef417153ca14 # bad: [ab3d681e9d41816f90836ea8fe235168d973207f] Merge branch 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad ab3d681e9d41816f90836ea8fe235168d973207f # good: [14961444696effb2e660fe876e5c1880f8bc3932] rcu: Shrink TINY_RCU by reworking CPU-stall ifdefs git bisect good 14961444696effb2e660fe876e5c1880f8bc3932 # good: [be77f87c001b770f13fe742becb08b847d9542f1] Merge branches 'cbnum.2013.06.10a', 'doc.2013.06.10a', 'fixes.2013.06.10a', 'srcu.2013.06.10a' and 'tiny.2013.06.10a' into HEAD git bisect good be77f87c001b770f13fe742becb08b847d9542f1 # bad: [2fe3d4b149ccebbb384062fbbe6634439f2bf120] mutex: Add more tests to lib/locking-selftest.c git bisect bad 2fe3d4b149ccebbb384062fbbe6634439f2bf120 # bad: [040a0a37100563754bb1fee6ff6427420bcfa609] mutex: Add support for wound/wait style locks git bisect bad 040a0a37100563754bb1fee6ff6427420bcfa609 # good: [a41b56efa70e060f650aeb54740aaf52044a1ead] arch: Make __mutex_fastpath_lock_retval return whether fastpath succeeded or not git bisect good a41b56efa70e060f650aeb54740aaf52044a1ead ---------- bisection log end ---------- ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. 2013-09-07 16:00 [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel Tetsuo Handa @ 2013-09-08 4:32 ` Tetsuo Handa 2013-09-08 5:28 ` Ilia Mirkin 0 siblings, 1 reply; 15+ messages in thread From: Tetsuo Handa @ 2013-09-08 4:32 UTC (permalink / raw) To: maarten.lankhorst Cc: linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Hello. I found what is wrong. ---------- bad patch start ---------- >From 3c56dfbd32a9b67ba824ce96128bb513eb65de4b Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sun, 8 Sep 2013 12:44:20 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" which I guess the author meant that "__builtin_constant_p(p) && p", but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: <stable@kernel.org> [3.11+] --- kernel/mutex.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index a52ee7bb..0a6f14f 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (__builtin_constant_p(ww_ctx) && ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -548,7 +548,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -568,7 +568,7 @@ done: mutex_remove_waiter(lock, &waiter, current_thread_info()); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (__builtin_constant_p(ww_ctx) && ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); -- 1.7.8 ---------- bad patch end ---------- However, after applying the patch above, I get problems (both gcc 3.x and 4.x) with locking selftests. ---------- gcc version 3.3.5 start ---------- [ 0.000000] Linux version 3.11.0-dirty (root@aqua) (gcc version 3.3.5 (Debian 1:3.3.5-13)) #124 SMP Sun Sep 8 12:05:18 JST 2013 (...snipped...) [ 0.000000] -------------------------------------------------------------------------- [ 0.000000] | Wound/wait tests | [ 0.000000] --------------------- [ 0.000000] ww api failures: [ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at lib/locking-selftest.c:1143 ww_test_fail_acquire+0x112/0x2c0() [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000477 00000000 c1577f18 c11e6736 c1577f24 c11e677d c11ffeb2 c1577f50 [ 0.000000] c1041af9 c14f5ea0 00000000 00000000 c15114fa 00000477 c11ffeb2 c1cd9360 [ 0.000000] 00000000 00000001 c1577f60 c1041bbd 00000009 00000000 c1577f84 c11ffeb2 [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c11ffeb2>] ? ww_test_fail_acquire+0x112/0x2c0 [ 0.000000] [<c1041af9>] warn_slowpath_common+0x79/0xa0 [ 0.000000] [<c11ffeb2>] ? ww_test_fail_acquire+0x112/0x2c0 [ 0.000000] [<c1041bbd>] warn_slowpath_null+0x1d/0x30 [ 0.000000] [<c11ffeb2>] ww_test_fail_acquire+0x112/0x2c0 [ 0.000000] [<c11ffce2>] ? dotest+0x42/0x100 [ 0.000000] [<c11ffda0>] ? dotest+0x100/0x100 [ 0.000000] [<c11ffce2>] dotest+0x42/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c12030b3>] ww_tests+0x53/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] ---[ end trace 74d4202eb2b56266 ]--- [ 0.000000] ok | ok | ok | [ 0.000000] ww contexts mixing: ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1577f78 c11e6736 c1577f84 c11e677d c1200480 c1577fac [ 0.000000] c11ffd01 c151157d c1084f95 00000000 ffffffff 00000010 00000000 00020800 [ 0.000000] c1788800 c1577fbc c120311b c15115f7 c151160d c1577fd0 c1204e14 c1504b06 [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c1200480>] ? ww_test_two_contexts+0x160/0x160 [ 0.000000] [<c11ffd01>] dotest+0x61/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c120311b>] ww_tests+0xbb/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] [ 0.000000] finishing ww context: ok | ok | ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1577f78 c11e6736 c1577f84 c11e677d c1200ba0 c1577fac [ 0.000000] c11ffd01 c151157d c1084f95 00000000 ffffffff 00000010 00000000 00020800 [ 0.000000] c1788800 c1577fbc c1203180 c15115f7 c1511620 c1577fd0 c1204e14 c1504b06 [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c1200ba0>] ? ww_test_context_fini_early+0x1e0/0x1e0 [ 0.000000] [<c11ffd01>] dotest+0x61/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c1203180>] ww_tests+0x120/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] [ 0.000000] locking mismatches: ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1577f78 c11e6736 c1577f84 c11e677d c1200d40 c1577fac [ 0.000000] c11ffd01 c151157d c1084f95 00000000 ffffffff 00000010 00000000 00020800 [ 0.000000] c1788800 c1577fbc c12031c3 c15115f7 c1511635 c1577fd0 c1204e14 c1504b06 [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c1200d40>] ? ww_test_object_unlock_twice+0x30/0x30 [ 0.000000] [<c11ffd01>] dotest+0x61/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c12031c3>] ww_tests+0x163/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1577f78 c11e6736 c1577f84 c11e677d c1200ea0 c1577fac [ 0.000000] c11ffd01 c151157d c1084f95 00000000 ffffffff 00000010 00000000 00020800 [ 0.000000] c1788800 c1577fbc c12031d4 c15115f7 c1511635 c1577fd0 c1204e14 c1504b06 [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c1200ea0>] ? ww_test_object_lock_unbalanced+0x160/0x160 [ 0.000000] [<c11ffd01>] dotest+0x61/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c12031d4>] ww_tests+0x174/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] [ 0.000000] EDEADLK handling: [ 0.000000] BUG: scheduling while atomic: swapper/0/0/0x00000002 [ 0.000000] 2 locks held by swapper/0/0: [ 0.000000] #0: (ww_lockdep_acquire){+.+...}, at: [<c11ffce2>] dotest+0x42/0x100 [ 0.000000] #1: (ww_lockdep_mutex){+.+...}, at: [<c1201093>] ww_test_edeadlk_normal+0x103/0x210 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00200246 00000000 c1577e04 c11e6736 c1577e10 c11e677d c1581160 c1577e2c [ 0.000000] c106cd05 c14f7440 c1581430 00000000 00000002 00000000 c1577efc c13c0a57 [ 0.000000] c17b56a8 00000001 c1581608 c1577e80 c109363e 00000000 c1577e78 c100470e [ 0.000000] Call Trace: [ 0.000000] [<c11e6736>] __dump_stack+0x16/0x20 [ 0.000000] [<c11e677d>] dump_stack+0x3d/0x60 [ 0.000000] [<c106cd05>] __schedule_bug+0x75/0xa0 [ 0.000000] [<c13c0a57>] __schedule+0x67/0x790 [ 0.000000] [<c109363e>] ? validate_chain+0x49e/0x540 [ 0.000000] [<c100470e>] ? dump_trace+0x9e/0xd0 [ 0.000000] [<c1090ab9>] ? save_trace+0x39/0xa0 [ 0.000000] [<c109438a>] ? mark_held_locks+0xca/0x100 [ 0.000000] [<c13bfb2c>] ? __ww_mutex_lock+0x1cc/0x380 [ 0.000000] [<c10945ce>] ? trace_hardirqs_on_caller+0x14e/0x160 [ 0.000000] [<c13c11e5>] schedule+0x65/0x70 [ 0.000000] [<c13c1202>] schedule_preempt_disabled+0x12/0x20 [ 0.000000] [<c13bfb33>] __ww_mutex_lock+0x1d3/0x380 [ 0.000000] [<c12010e3>] ? ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c12010e3>] ? ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c12010e3>] ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c11ffce2>] ? dotest+0x42/0x100 [ 0.000000] [<c1200f90>] ? ww_test_object_lock_stale_context+0xf0/0xf0 [ 0.000000] [<c11ffce2>] dotest+0x42/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c1203209>] ww_tests+0x1a9/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] BUG: unable to handle kernel NULL pointer dereference at 00000008 [ 0.000000] IP: [<c11ebce3>] rb_next+0x3/0x60 [ 0.000000] *pde = 00000000 [ 0.000000] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #124 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] task: c1581160 ti: c1576000 task.ti: c1576000 [ 0.000000] EIP: 0060:[<c11ebce3>] EFLAGS: 00210002 CPU: 0 [ 0.000000] EIP is at rb_next+0x3/0x60 [ 0.000000] EAX: 00000008 EBX: dffed47c ECX: fffffff8 EDX: 00000008 [ 0.000000] ESI: 00000000 EDI: c15813e0 EBP: c1577e04 ESP: c1577dfc [ 0.000000] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.000000] CR0: 8005003b CR2: 00000008 CR3: 01787000 CR4: 00000690 [ 0.000000] Stack: [ 0.000000] c1577e04 c1073ceb c1577e1c c10759a5 00000000 dffed420 dffed47c c15813e0 [ 0.000000] c1577e2c c1077937 dffed420 ffffffff c1577efc c13c0bf5 c17b56a8 00000001 [ 0.000000] c1581608 c1577e80 c109363e 00000000 c1577e78 c100470e c13c6f68 c1905be0 [ 0.000000] Call Trace: [ 0.000000] [<c1073ceb>] ? __pick_next_entity+0xb/0x20 [ 0.000000] [<c10759a5>] pick_next_entity+0x25/0xa0 [ 0.000000] [<c1077937>] pick_next_task_fair+0x27/0x40 [ 0.000000] [<c13c0bf5>] __schedule+0x205/0x790 [ 0.000000] [<c109363e>] ? validate_chain+0x49e/0x540 [ 0.000000] [<c100470e>] ? dump_trace+0x9e/0xd0 [ 0.000000] [<c1090ab9>] ? save_trace+0x39/0xa0 [ 0.000000] [<c109438a>] ? mark_held_locks+0xca/0x100 [ 0.000000] [<c13bfb2c>] ? __ww_mutex_lock+0x1cc/0x380 [ 0.000000] [<c10945ce>] ? trace_hardirqs_on_caller+0x14e/0x160 [ 0.000000] [<c13c11e5>] schedule+0x65/0x70 [ 0.000000] [<c13c1202>] schedule_preempt_disabled+0x12/0x20 [ 0.000000] [<c13bfb33>] __ww_mutex_lock+0x1d3/0x380 [ 0.000000] [<c12010e3>] ? ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c12010e3>] ? ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c12010e3>] ww_test_edeadlk_normal+0x153/0x210 [ 0.000000] [<c11ffce2>] ? dotest+0x42/0x100 [ 0.000000] [<c1200f90>] ? ww_test_object_lock_stale_context+0xf0/0xf0 [ 0.000000] [<c11ffce2>] dotest+0x42/0x100 [ 0.000000] [<c1084f95>] ? printk+0x35/0x40 [ 0.000000] [<c1203209>] ww_tests+0x1a9/0x410 [ 0.000000] [<c1204e14>] locking_selftest+0x19a4/0x1ab0 [ 0.000000] [<c15c2a0c>] start_kernel+0x1ec/0x2c0 [ 0.000000] [<c15c2480>] ? repair_env_string+0x70/0x70 [ 0.000000] [<c15c2275>] i386_start_kernel+0x25/0x30 [ 0.000000] Code: d2 74 20 8b 42 04 85 c0 74 17 8d b4 26 00 00 00 00 8d bc 27 00 00 00 00 89 c2 8b 40 04 85 c0 75 f7 89 d0 5d c3 8d 76 00 55 89 c2 <8b> 08 31 c0 89 e5 39 d1 74 43 8b 42 04 85 c0 74 1c 89 c2 8b 40 [ 0.000000] EIP: [<c11ebce3>] rb_next+0x3/0x60 SS:ESP 0068:c1577dfc [ 0.000000] CR2: 0000000000000008 [ 0.000000] ---[ end trace 74d4202eb2b56267 ]--- [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! ---------- gcc version 3.3.5 end ---------- ---------- gcc version 4.6.3 start ---------- [ 0.000000] Linux version 3.11.0-dirty (root@aqua) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #125 SMP Sun Sep 8 12:13:43 JST 2013 (...snipped...) [ 0.000000] -------------------------------------------------------------------------- [ 0.000000] | Wound/wait tests | [ 0.000000] --------------------- [ 0.000000] ww api failures: [ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at lib/locking-selftest.c:1143 ww_test_fail_acquire+0xee/0x280() [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505f2c c13938a3 c14a89d9 c1505f5c c103a47a c1492b9c [ 0.000000] 00000000 00000000 c14a89d9 00000477 c11e16de c11e16de c11e15f0 00000001 [ 0.000000] 00000001 c1505f6c c103a4bd 00000009 00000000 c1505f84 c11e16de 00000000 [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c103a47a>] warn_slowpath_common+0x7a/0xa0 [ 0.000000] [<c11e16de>] ? ww_test_fail_acquire+0xee/0x280 [ 0.000000] [<c11e16de>] ? ww_test_fail_acquire+0xee/0x280 [ 0.000000] [<c11e15f0>] ? ww_test_edeadlk_no_unlock_slow+0x270/0x270 [ 0.000000] [<c103a4bd>] warn_slowpath_null+0x1d/0x20 [ 0.000000] [<c11e16de>] ww_test_fail_acquire+0xee/0x280 [ 0.000000] [<c1393e84>] ? dotest+0x32/0xcf [ 0.000000] [<c1393e84>] dotest+0x32/0xcf [ 0.000000] [<c1393f67>] ww_tests+0x46/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] ---[ end trace 614b89df0eea1b4a ]--- [ 0.000000] ok | ok | ok | [ 0.000000] ww contexts mixing: ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505f84 c13938a3 c11e29a0 c1505fac c1393ea3 c14a89f0 [ 0.000000] 00000000 c14a893c c1505fa8 00000010 c157e100 00020800 c1707800 c1505fb8 [ 0.000000] c1393fc4 c14a4e60 c1505fcc c11e464c c14a4e60 c1495b4c 00000780 c1505fec [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c11e29a0>] ? ww_test_try_context+0x110/0x110 [ 0.000000] [<c1393ea3>] dotest+0x51/0xcf [ 0.000000] [<c1393fc4>] ww_tests+0xa3/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] [ 0.000000] finishing ww context: ok | ok | ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505f84 c13938a3 c11e0b70 c1505fac c1393ea3 c14a89f0 [ 0.000000] 00000000 c14a893c c1505fa8 00000010 c157e100 00020800 c1707800 c1505fb8 [ 0.000000] c139401e c14a4e60 c1505fcc c11e464c c14a4e60 c1495b4c 00000780 c1505fec [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c11e0b70>] ? ww_test_context_fini_early+0x1c0/0x1c0 [ 0.000000] [<c1393ea3>] dotest+0x51/0xcf [ 0.000000] [<c139401e>] ww_tests+0xfd/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] [ 0.000000] locking mismatches: ok |FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505f84 c13938a3 c11e1870 c1505fac c1393ea3 c14a89f0 [ 0.000000] 00000000 c14a893c c1505fa8 00000010 c157e100 00020800 c1707800 c1505fb8 [ 0.000000] c1394056 c14a4e60 c1505fcc c11e464c c14a4e60 c1495b4c 00000780 c1505fec [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c11e1870>] ? ww_test_fail_acquire+0x280/0x280 [ 0.000000] [<c1393ea3>] dotest+0x51/0xcf [ 0.000000] [<c1394056>] ww_tests+0x135/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] FAILED| [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505f84 c13938a3 c11e2a80 c1505fac c1393ea3 c14a89f0 [ 0.000000] 00000000 c14a893c c1505fa8 00000010 c157e100 00020800 c1707800 c1505fb8 [ 0.000000] c1394067 c14a4e60 c1505fcc c11e464c c14a4e60 c1495b4c 00000780 c1505fec [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c11e2a80>] ? ww_test_diff_class+0xe0/0xe0 [ 0.000000] [<c1393ea3>] dotest+0x51/0xcf [ 0.000000] [<c1394067>] ww_tests+0x146/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] [ 0.000000] EDEADLK handling: [ 0.000000] BUG: scheduling while atomic: swapper/0/0/0x00000002 [ 0.000000] 2 locks held by swapper/0/0: [ 0.000000] #0: (ww_lockdep_acquire){+.+...}, at: [<c1393e84>] dotest+0x32/0xcf [ 0.000000] #1: (ww_lockdep_mutex){+.+...}, at: [<c11e0e51>] ww_test_edeadlk_normal+0x181/0x220 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505e6c c13938a3 c150e920 c1505e88 c138e482 c1493b04 [ 0.000000] c150ebf0 00000000 00000002 dffed440 c1505f10 c139a565 00000000 003c41e1 [ 0.000000] 00000000 c150edc8 c1505eec c150e920 00000006 00000001 c16fe440 c16fe440 [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c138e482>] __schedule_bug+0x5f/0x71 [ 0.000000] [<c139a565>] __schedule+0x65/0x5d0 [ 0.000000] [<c10896bf>] ? mark_held_locks+0xcf/0x100 [ 0.000000] [<c13994f5>] ? __ww_mutex_lock+0x1b5/0x370 [ 0.000000] [<c1089976>] ? trace_hardirqs_on_caller+0x146/0x170 [ 0.000000] [<c139ab9d>] schedule+0x4d/0x50 [ 0.000000] [<c139add3>] schedule_preempt_disabled+0x13/0x20 [ 0.000000] [<c1399505>] __ww_mutex_lock+0x1c5/0x370 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0cd0>] ? ww_test_context_lock_after_done+0x160/0x160 [ 0.000000] [<c11e0e77>] ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c1393e84>] ? dotest+0x32/0xcf [ 0.000000] [<c1393e84>] dotest+0x32/0xcf [ 0.000000] [<c1394091>] ww_tests+0x170/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/cpu/common.c:1371 warn_pre_alternatives+0x22/0x30() [ 0.000000] You're using static_cpu_has before alternatives have run! [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] 00000000 00000000 c1505d38 c13938a3 c1484b96 c1505d68 c103a47a c1488bf8 [ 0.000000] c1505d94 00000000 c1484b96 0000055b c100f502 c100f502 c1505e20 00000008 [ 0.000000] c1032a90 c1505d80 c103a51e 00000009 c1505d78 c1488bf8 c1505d94 c1505d94 [ 0.000000] Call Trace: [ 0.000000] [<c13938a3>] dump_stack+0x4b/0x66 [ 0.000000] [<c103a47a>] warn_slowpath_common+0x7a/0xa0 [ 0.000000] [<c100f502>] ? warn_pre_alternatives+0x22/0x30 [ 0.000000] [<c100f502>] ? warn_pre_alternatives+0x22/0x30 [ 0.000000] [<c1032a90>] ? vmalloc_sync_all+0x120/0x120 [ 0.000000] [<c103a51e>] warn_slowpath_fmt+0x2e/0x30 [ 0.000000] [<c100f502>] warn_pre_alternatives+0x22/0x30 [ 0.000000] [<c1032655>] __do_page_fault+0xe5/0x400 [ 0.000000] [<c1088ee0>] ? validate_chain.isra.23+0x5d0/0x6c0 [ 0.000000] [<c106c5e0>] ? dequeue_entity+0x420/0x4f0 [ 0.000000] [<c1032a90>] ? vmalloc_sync_all+0x120/0x120 [ 0.000000] [<c1032a98>] do_page_fault+0x8/0x10 [ 0.000000] [<c139cb97>] error_code+0x5f/0x64 [ 0.000000] [<c1032a90>] ? vmalloc_sync_all+0x120/0x120 [ 0.000000] [<c11cc966>] ? rb_next+0x6/0x50 [ 0.000000] [<c106a0cd>] pick_next_entity+0x2d/0xb0 [ 0.000000] [<c106a16f>] pick_next_task_fair+0x1f/0x40 [ 0.000000] [<c139a725>] __schedule+0x225/0x5d0 [ 0.000000] [<c10896bf>] ? mark_held_locks+0xcf/0x100 [ 0.000000] [<c13994f5>] ? __ww_mutex_lock+0x1b5/0x370 [ 0.000000] [<c1089976>] ? trace_hardirqs_on_caller+0x146/0x170 [ 0.000000] [<c139ab9d>] schedule+0x4d/0x50 [ 0.000000] [<c139add3>] schedule_preempt_disabled+0x13/0x20 [ 0.000000] [<c1399505>] __ww_mutex_lock+0x1c5/0x370 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0cd0>] ? ww_test_context_lock_after_done+0x160/0x160 [ 0.000000] [<c11e0e77>] ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c1393e84>] ? dotest+0x32/0xcf [ 0.000000] [<c1393e84>] dotest+0x32/0xcf [ 0.000000] [<c1394091>] ww_tests+0x170/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] ---[ end trace 614b89df0eea1b4b ]--- [ 0.000000] BUG: unable to handle kernel NULL pointer dereference at 00000008 [ 0.000000] IP: [<c11cc966>] rb_next+0x6/0x50 [ 0.000000] *pde = 00000000 [ 0.000000] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.11.0-dirty #125 [ 0.000000] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007 [ 0.000000] task: c150e920 ti: c1504000 task.ti: c1504000 [ 0.000000] EIP: 0060:[<c11cc966>] EFLAGS: 00210046 CPU: 0 [ 0.000000] EIP is at rb_next+0x6/0x50 [ 0.000000] EAX: 00000008 EBX: dffed4a0 ECX: 00000000 EDX: fffffff8 [ 0.000000] ESI: 00000000 EDI: 00000000 EBP: c1505e60 ESP: c1505e5c [ 0.000000] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.000000] CR0: 8005003b CR2: 00000008 CR3: 01706000 CR4: 00000690 [ 0.000000] Stack: [ 0.000000] dffed4a0 c1505e78 c106a0cd 0112a880 00000000 dffed4a0 00000000 c1505e88 [ 0.000000] c106a16f dffed440 c150eba0 c1505f10 c139a725 00000000 003c41e1 00000000 [ 0.000000] c150edc8 c1505eec c150e920 00000006 00000001 c16fe440 c16fe440 c150e920 [ 0.000000] Call Trace: [ 0.000000] [<c106a0cd>] pick_next_entity+0x2d/0xb0 [ 0.000000] [<c106a16f>] pick_next_task_fair+0x1f/0x40 [ 0.000000] [<c139a725>] __schedule+0x225/0x5d0 [ 0.000000] [<c10896bf>] ? mark_held_locks+0xcf/0x100 [ 0.000000] [<c13994f5>] ? __ww_mutex_lock+0x1b5/0x370 [ 0.000000] [<c1089976>] ? trace_hardirqs_on_caller+0x146/0x170 [ 0.000000] [<c139ab9d>] schedule+0x4d/0x50 [ 0.000000] [<c139add3>] schedule_preempt_disabled+0x13/0x20 [ 0.000000] [<c1399505>] __ww_mutex_lock+0x1c5/0x370 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0e77>] ? ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c11e0cd0>] ? ww_test_context_lock_after_done+0x160/0x160 [ 0.000000] [<c11e0e77>] ww_test_edeadlk_normal+0x1a7/0x220 [ 0.000000] [<c1393e84>] ? dotest+0x32/0xcf [ 0.000000] [<c1393e84>] dotest+0x32/0xcf [ 0.000000] [<c1394091>] ww_tests+0x170/0x398 [ 0.000000] [<c11e464c>] locking_selftest+0x168c/0x1720 [ 0.000000] [<c154a939>] start_kernel+0x249/0x325 [ 0.000000] [<c154a52e>] ? obsolete_checksetup+0x95/0x95 [ 0.000000] [<c154a358>] i386_start_kernel+0x12e/0x131 [ 0.000000] Code: 90 8d 74 26 00 55 8b 00 89 e5 85 c0 75 09 eb 0e 90 8d 74 26 00 89 d0 8b 50 04 85 d2 75 f7 5d c3 90 8d 74 26 00 55 31 c9 89 e5 53 <8b> 10 39 d0 74 3c 8b 48 04 85 c9 75 07 eb 13 8d 76 00 89 d1 8b [ 0.000000] EIP: [<c11cc966>] rb_next+0x6/0x50 SS:ESP 0068:c1505e5c [ 0.000000] CR2: 0000000000000008 [ 0.000000] ---[ end trace 614b89df0eea1b4c ]--- [ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task! ---------- gcc version 4.6.3 end ---------- The patch below can fix "boot failure on gcc 3.x" and avoid "locking selftests failure on gcc 3.x / 4.x". ---------- good patch start ---------- >From 04a2739f00822a4ca59128501b1f3f5bf4981af7 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sun, 8 Sep 2013 13:06:42 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. But changing from "!__builtin_constant_p(p == NULL)" to "__builtin_constant_p(p) && p" causes locking selftest failures when built with CONFIG_DEBUG_LOCKING_API_SELFTESTS=y. Therefore, change from "!__builtin_constant_p(p == NULL)" to "p". Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: <stable@kernel.org> [3.11+] --- kernel/mutex.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index a52ee7bb..3b04d6a 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -548,7 +548,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -568,7 +568,7 @@ done: mutex_remove_waiter(lock, &waiter, current_thread_info()); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); -- 1.7.8 ---------- good patch end ---------- We need to give up __builtin_constant_p() optimization after all? ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. 2013-09-08 4:32 ` Tetsuo Handa @ 2013-09-08 5:28 ` Ilia Mirkin 2013-09-08 7:24 ` Tetsuo Handa 0 siblings, 1 reply; 15+ messages in thread From: Ilia Mirkin @ 2013-09-08 5:28 UTC (permalink / raw) To: Tetsuo Handa Cc: Maarten Lankhorst, linux-kernel@vger.kernel.org, Daniel Vetter, Rob Clark, a.p.zijlstra, mingo On Sun, Sep 8, 2013 at 12:32 AM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Hello. > > I found what is wrong. > > ---------- bad patch start ---------- > >From 3c56dfbd32a9b67ba824ce96128bb513eb65de4b Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sun, 8 Sep 2013 12:44:20 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" which I guess the author meant that > "__builtin_constant_p(p) && p", but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. I think that !__builtin_constant_p(p == NULL) is basically saying "I am unable to conclude that p == NULL at build time", which would translate to something along the lines of (__builtin_constant_p(p) && p) || !__builtin_constant_p(p) Your logic will be be false for non-built-in-constants supplied as p. Or perhaps it's just equivalent to !__builtin_constant_p(p), since the compiler's ability to conclude whether it is NULL at build-time should be unaffected by whether it actually is NULL or not. Some simple experimentation with recent gcc's should be able to determine this. The more I think about it, the more likely the latter interpretation is correct and you can just drop the == NULL's. (Although perhaps the original intent was more like the former.) -ilia ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. 2013-09-08 5:28 ` Ilia Mirkin @ 2013-09-08 7:24 ` Tetsuo Handa 2013-09-08 7:42 ` Ilia Mirkin 2013-09-08 8:11 ` Maarten Lankhorst 0 siblings, 2 replies; 15+ messages in thread From: Tetsuo Handa @ 2013-09-08 7:24 UTC (permalink / raw) To: imirkin, maarten.lankhorst Cc: linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Hello. Ilia Mirkin wrote: > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > > "!__builtin_constant_p(p == NULL)" which I guess the author meant that > > "__builtin_constant_p(p) && p", but gcc 3.x cannot handle such expression > > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > I think that !__builtin_constant_p(p == NULL) is basically saying "I > am unable to conclude that p == NULL at build time", which would > translate to something along the lines of > > (__builtin_constant_p(p) && p) || !__builtin_constant_p(p) > I think (__builtin_constant_p(p) && p) && p->acquired > 0 is safe but (!__builtin_constant_p(p)) && p->acquired > 0 is not safe, for "p != NULL" check is required for avoiding NULL pointer dereference. It seems to me that (!__builtin_constant_p(p == NULL)) need to be translated to something along the lines of (__builtin_constant_p(p) && p) || (!__builtin_constant_p(p) && p) which can be simplified as (p) . > Or perhaps it's just equivalent to !__builtin_constant_p(p), since the > compiler's ability to conclude whether it is NULL at build-time should > be unaffected by whether it actually is NULL or not. Likewise, it seems to me that (!__builtin_constant_p(p == NULL)) need to be translated to something along the lines of (!__builtin_constant_p(p) && p) . Well this change as well can fix "boot failure on gcc 3.x" and avoid "locking selftests failure on gcc 3.x / 4.x". OK, let's wait for answer from the author. Can I add "Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>" to below patch? ---------- good patch start ---------- >From a8bbf6b3c2d44cb90d63820f146aaff119d871c9 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sun, 8 Sep 2013 16:09:27 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Fix it by changing from "!__builtin_constant_p(p == NULL)" to "!__builtin_constant_p(p) && p". Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: <stable@kernel.org> [3.11+] --- kernel/mutex.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index a52ee7bb..ef02003 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -548,7 +548,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -568,7 +568,7 @@ done: mutex_remove_waiter(lock, &waiter, current_thread_info()); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); -- 1.7.8 ---------- good patch end ---------- ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. 2013-09-08 7:24 ` Tetsuo Handa @ 2013-09-08 7:42 ` Ilia Mirkin 2013-09-08 8:11 ` Maarten Lankhorst 1 sibling, 0 replies; 15+ messages in thread From: Ilia Mirkin @ 2013-09-08 7:42 UTC (permalink / raw) To: Tetsuo Handa Cc: Maarten Lankhorst, linux-kernel@vger.kernel.org, Daniel Vetter, Rob Clark, a.p.zijlstra, mingo On Sun, Sep 8, 2013 at 3:24 AM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Hello. > > Ilia Mirkin wrote: >> > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used >> > "!__builtin_constant_p(p == NULL)" which I guess the author meant that >> > "__builtin_constant_p(p) && p", but gcc 3.x cannot handle such expression >> > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. >> >> I think that !__builtin_constant_p(p == NULL) is basically saying "I >> am unable to conclude that p == NULL at build time", which would >> translate to something along the lines of >> >> (__builtin_constant_p(p) && p) || !__builtin_constant_p(p) >> > > I think > > (__builtin_constant_p(p) && p) && p->acquired > 0 > > is safe but > > (!__builtin_constant_p(p)) && p->acquired > 0 > > is not safe, for "p != NULL" check is required for avoiding NULL pointer > dereference. > > It seems to me that > > (!__builtin_constant_p(p == NULL)) > > need to be translated to something along the lines of > > (__builtin_constant_p(p) && p) || (!__builtin_constant_p(p) && p) > > which can be simplified as > > (p) > > . > >> Or perhaps it's just equivalent to !__builtin_constant_p(p), since the >> compiler's ability to conclude whether it is NULL at build-time should >> be unaffected by whether it actually is NULL or not. > > Likewise, it seems to me that > > (!__builtin_constant_p(p == NULL)) > > need to be translated to something along the lines of > > (!__builtin_constant_p(p) && p) Well, I think the theory is that if p is not a compile-time constant then it must not be null. At least that's the implication of the current code. As I understand it, the == NULL is a no-op as far as gcc is concerned, since it's not evaluating the expr, only checking if it can be evaluated. Unless there can be a situation where it can know whether a value is null or not, but not know its actual value, in which case the && p is warranted. > > . Well this change as well can fix "boot failure on gcc 3.x" and avoid "locking > selftests failure on gcc 3.x / 4.x". OK, let's wait for answer from the author. Probably best to do that, yes. Maarten? > > Can I add "Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>" to below patch? I don't think that's the correct usage of "Signed-off-by", since I neither wrote the patch nor am I on the upstream path. If you really want to give credit, you could invent a "Suggested-by" tag, but I really don't care. > > ---------- good patch start ---------- > >From a8bbf6b3c2d44cb90d63820f146aaff119d871c9 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sun, 8 Sep 2013 16:09:27 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Fix it by changing from "!__builtin_constant_p(p == NULL)" to > "!__builtin_constant_p(p) && p". > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: <stable@kernel.org> [3.11+] > --- > kernel/mutex.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/kernel/mutex.c b/kernel/mutex.c > index a52ee7bb..ef02003 100644 > --- a/kernel/mutex.c > +++ b/kernel/mutex.c > @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct task_struct *owner; > struct mspin_node node; > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > if ((atomic_read(&lock->count) == 1) && > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > lock_acquired(&lock->dep_map, ip); > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { > struct ww_mutex *ww; > ww = container_of(lock, struct ww_mutex, base); > > @@ -548,7 +548,7 @@ slowpath: > goto err; > } > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { > ret = __mutex_lock_check_stamp(lock, ww_ctx); > if (ret) > goto err; > @@ -568,7 +568,7 @@ done: > mutex_remove_waiter(lock, &waiter, current_thread_info()); > mutex_set_owner(lock); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { > struct ww_mutex *ww = container_of(lock, > struct ww_mutex, > base); > -- > 1.7.8 > ---------- good patch end ---------- ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel. 2013-09-08 7:24 ` Tetsuo Handa 2013-09-08 7:42 ` Ilia Mirkin @ 2013-09-08 8:11 ` Maarten Lankhorst 2013-09-08 11:53 ` [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel Tetsuo Handa 1 sibling, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2013-09-08 8:11 UTC (permalink / raw) To: Tetsuo Handa Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Op 08-09-13 09:24, Tetsuo Handa schreef: > Hello. > > Ilia Mirkin wrote: >>> Commit 040a0a37 "mutex: Add support for wound/wait style locks" used >>> "!__builtin_constant_p(p == NULL)" which I guess the author meant that >>> "__builtin_constant_p(p) && p", but gcc 3.x cannot handle such expression >>> correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. >> I think that !__builtin_constant_p(p == NULL) is basically saying "I >> am unable to conclude that p == NULL at build time", which would >> translate to something along the lines of >> >> (__builtin_constant_p(p) && p) || !__builtin_constant_p(p) >> > I think > > (__builtin_constant_p(p) && p) && p->acquired > 0 > > is safe but > > (!__builtin_constant_p(p)) && p->acquired > 0 > > is not safe, for "p != NULL" check is required for avoiding NULL pointer > dereference. > > It seems to me that > > (!__builtin_constant_p(p == NULL)) > > need to be translated to something along the lines of > > (__builtin_constant_p(p) && p) || (!__builtin_constant_p(p) && p) > > which can be simplified as > > (p) > > . > >> Or perhaps it's just equivalent to !__builtin_constant_p(p), since the >> compiler's ability to conclude whether it is NULL at build-time should >> be unaffected by whether it actually is NULL or not. > Likewise, it seems to me that > > (!__builtin_constant_p(p == NULL)) > > need to be translated to something along the lines of > > (!__builtin_constant_p(p) && p) > > . Well this change as well can fix "boot failure on gcc 3.x" and avoid "locking > selftests failure on gcc 3.x / 4.x". OK, let's wait for answer from the author. > > Can I add "Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>" to below patch? > > ---------- good patch start ---------- > >From a8bbf6b3c2d44cb90d63820f146aaff119d871c9 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sun, 8 Sep 2013 16:09:27 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Fix it by changing from "!__builtin_constant_p(p == NULL)" to > "!__builtin_constant_p(p) && p". > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: <stable@kernel.org> [3.11+] > --- > kernel/mutex.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/kernel/mutex.c b/kernel/mutex.c > index a52ee7bb..ef02003 100644 > --- a/kernel/mutex.c > +++ b/kernel/mutex.c > @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct task_struct *owner; > struct mspin_node node; > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > if ((atomic_read(&lock->count) == 1) && > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > lock_acquired(&lock->dep_map, ip); > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { > struct ww_mutex *ww; > ww = container_of(lock, struct ww_mutex, base); > > @@ -548,7 +548,7 @@ slowpath: > goto err; > } > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx && ww_ctx->acquired > 0) { > ret = __mutex_lock_check_stamp(lock, ww_ctx); > if (ret) > goto err; > @@ -568,7 +568,7 @@ done: > mutex_remove_waiter(lock, &waiter, current_thread_info()); > mutex_set_owner(lock); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (!__builtin_constant_p(ww_ctx) && ww_ctx) { > struct ww_mutex *ww = container_of(lock, > struct ww_mutex, > base); Wrong. Wrong. Wrong. The builtin_constant_p was explicitly added to NOT do a null pointer check in the ww_ctx case, and now you re-introduce it for ALL compilers. Gcc will still think ww_ctx may be NULL in the ww_mutex_lock case. __builtin_constant_p(ww_ctx) always evaluates to false, and is not equivalent to __builtin_constant_p(ww_ctx != NULL) in gcc 4.6 at least, I have no idea why gcc treats pointers differently like that. Explicitly testing against NULL fixes it. __builtin_constant_p(ww_ctx == NULL) should return the same value as __builtin_constant_p(ww_ctx != NULL), but I did the == NULL check for clarity, if it's broken for your compiler, please add a bool use_ww_ctx or something to __mutex_lock_common that's set directly instead, the __builtin_constant_p trick might be too gcc version specific. ~Maarten ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-08 8:11 ` Maarten Lankhorst @ 2013-09-08 11:53 ` Tetsuo Handa 2013-09-08 20:36 ` Maarten Lankhorst 0 siblings, 1 reply; 15+ messages in thread From: Tetsuo Handa @ 2013-09-08 11:53 UTC (permalink / raw) To: maarten.lankhorst Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Hello. Maarten Lankhorst wrote: > if it's broken for your compiler, please add a bool use_ww_ctx or something to __mutex_lock_common that's set directly instead, the __builtin_constant_p trick > might be too gcc version specific. I see. I tested that both gcc 3.x and gcc 4.x can generate bootable kernel using below fix. ---------- >From f71fb89bccaa7ed5b3a14e735a1b9cc1a0e7112d Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Sun, 8 Sep 2013 20:37:19 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: <stable@kernel.org> [3.11+] --- kernel/mutex.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index a52ee7bb..2e7984e 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -414,6 +414,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct mutex_waiter waiter; unsigned long flags; int ret; + const bool use_ww_ctx = !!ww_ctx; preempt_disable(); mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip); @@ -448,7 +449,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -478,7 +479,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -548,7 +549,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -568,7 +569,7 @@ done: mutex_remove_waiter(lock, &waiter, current_thread_info()); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); -- 1.7.8 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-08 11:53 ` [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel Tetsuo Handa @ 2013-09-08 20:36 ` Maarten Lankhorst 2013-09-09 11:56 ` Tetsuo Handa 0 siblings, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2013-09-08 20:36 UTC (permalink / raw) To: Tetsuo Handa Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Op 08-09-13 13:53, Tetsuo Handa schreef: > Hello. > > Maarten Lankhorst wrote: >> if it's broken for your compiler, please add a bool use_ww_ctx or something to __mutex_lock_common that's set directly instead, the __builtin_constant_p trick >> might be too gcc version specific. > I see. I tested that both gcc 3.x and gcc 4.x can generate bootable kernel > using below fix. > ---------- > >From f71fb89bccaa7ed5b3a14e735a1b9cc1a0e7112d Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Sun, 8 Sep 2013 20:37:19 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() > usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: <stable@kernel.org> [3.11+] > --- > kernel/mutex.c | 9 +++++---- > 1 files changed, 5 insertions(+), 4 deletions(-) Almost correct. I meant passing it as parameter to __mutex_lock_common. Your version will still cause an extra pointless null check in the ww_mutex_lock case. > diff --git a/kernel/mutex.c b/kernel/mutex.c > index a52ee7bb..2e7984e 100644 > --- a/kernel/mutex.c > +++ b/kernel/mutex.c > @@ -414,6 +414,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct mutex_waiter waiter; > unsigned long flags; > int ret; > + const bool use_ww_ctx = !!ww_ctx; > > preempt_disable(); > mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip); > @@ -448,7 +449,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct task_struct *owner; > struct mspin_node node; > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > @@ -478,7 +479,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > if ((atomic_read(&lock->count) == 1) && > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > lock_acquired(&lock->dep_map, ip); > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww; > ww = container_of(lock, struct ww_mutex, base); > > @@ -548,7 +549,7 @@ slowpath: > goto err; > } > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > ret = __mutex_lock_check_stamp(lock, ww_ctx); > if (ret) > goto err; > @@ -568,7 +569,7 @@ done: > mutex_remove_waiter(lock, &waiter, current_thread_info()); > mutex_set_owner(lock); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww = container_of(lock, > struct ww_mutex, > base); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-08 20:36 ` Maarten Lankhorst @ 2013-09-09 11:56 ` Tetsuo Handa 2013-09-09 12:07 ` Maarten Lankhorst 2013-09-09 12:58 ` Peter Zijlstra 0 siblings, 2 replies; 15+ messages in thread From: Tetsuo Handa @ 2013-09-09 11:56 UTC (permalink / raw) To: maarten.lankhorst Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Maarten Lankhorst wrote: > Almost correct. I meant passing it as parameter to __mutex_lock_common. Your version will still cause an extra pointless null check in the ww_mutex_lock case. Ah, I see. ---------- >From 95f189eb37c25ddf8e48d5dfc2f9f1185c52b6a8 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Mon, 9 Sep 2013 20:48:13 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Fix it by explicitly passing a bool which tells whether p != NULL or not. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: <stable@kernel.org> [3.11+] --- kernel/mutex.c | 32 ++++++++++++++++---------------- 1 files changed, 16 insertions(+), 16 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index a52ee7bb..a2b80f1 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -408,7 +408,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, static __always_inline int __sched __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct lockdep_map *nest_lock, unsigned long ip, - struct ww_acquire_ctx *ww_ctx) + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) { struct task_struct *task = current; struct mutex_waiter waiter; @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -548,7 +548,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -568,7 +568,7 @@ done: mutex_remove_waiter(lock, &waiter, current_thread_info()); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); @@ -618,7 +618,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_nested); @@ -628,7 +628,7 @@ _mutex_lock_nest_lock(struct mutex *lock, struct lockdep_map *nest) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - 0, nest, _RET_IP_, NULL); + 0, nest, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(_mutex_lock_nest_lock); @@ -638,7 +638,7 @@ mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_KILLABLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_killable_nested); @@ -647,7 +647,7 @@ mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested); @@ -685,7 +685,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -700,7 +700,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -812,28 +812,28 @@ __mutex_lock_slowpath(atomic_t *lock_count) struct mutex *lock = container_of(lock_count, struct mutex, count); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_killable_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_KILLABLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_interruptible_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __ww_mutex_lock_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } static noinline int __sched @@ -841,7 +841,7 @@ __ww_mutex_lock_interruptible_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } #endif -- 1.7.8 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-09 11:56 ` Tetsuo Handa @ 2013-09-09 12:07 ` Maarten Lankhorst 2013-09-09 13:22 ` Tetsuo Handa 2013-09-09 12:58 ` Peter Zijlstra 1 sibling, 1 reply; 15+ messages in thread From: Maarten Lankhorst @ 2013-09-09 12:07 UTC (permalink / raw) To: Tetsuo Handa Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Op 09-09-13 13:56, Tetsuo Handa schreef: > Maarten Lankhorst wrote: >> Almost correct. I meant passing it as parameter to __mutex_lock_common. Your version will still cause an extra pointless null check in the ww_mutex_lock case. > Ah, I see. > ---------- > >From 95f189eb37c25ddf8e48d5dfc2f9f1185c52b6a8 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Mon, 9 Sep 2013 20:48:13 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Fix it by explicitly passing a bool which tells whether p != NULL or not. > Yeah looks ok, did you run the selftests from CONFIG_DEBUG_LOCKING_API_SELFTESTS, with/without CONFIG_PROVE_LOCKING and once more with DEBUG_MUTEXES also unset? ~Maarten ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-09 12:07 ` Maarten Lankhorst @ 2013-09-09 13:22 ` Tetsuo Handa 2013-09-24 14:51 ` Tetsuo Handa 0 siblings, 1 reply; 15+ messages in thread From: Tetsuo Handa @ 2013-09-09 13:22 UTC (permalink / raw) To: maarten.lankhorst Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Maarten Lankhorst wrote: > Yeah looks ok, did you run the selftests from CONFIG_DEBUG_LOCKING_API_SELFTESTS, > with/without CONFIG_PROVE_LOCKING and once more with DEBUG_MUTEXES also unset? Since CONFIG_DEBUG_MUTEXES=n && CONFIG_PROVE_LOCKING=y is impossible, I tested CONFIG_DEBUG_MUTEXES=y CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_LOCKING_API_SELFTESTS=y CONFIG_DEBUG_MUTEXES=y CONFIG_PROVE_LOCKING=n CONFIG_DEBUG_LOCKING_API_SELFTESTS=y CONFIG_DEBUG_MUTEXES=n CONFIG_PROVE_LOCKING=n CONFIG_DEBUG_LOCKING_API_SELFTESTS=y CONFIG_DEBUG_MUTEXES=y CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_LOCKING_API_SELFTESTS=n CONFIG_DEBUG_MUTEXES=y CONFIG_PROVE_LOCKING=n CONFIG_DEBUG_LOCKING_API_SELFTESTS=n CONFIG_DEBUG_MUTEXES=n CONFIG_PROVE_LOCKING=n CONFIG_DEBUG_LOCKING_API_SELFTESTS=n and all works OK. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-09 13:22 ` Tetsuo Handa @ 2013-09-24 14:51 ` Tetsuo Handa 0 siblings, 0 replies; 15+ messages in thread From: Tetsuo Handa @ 2013-09-24 14:51 UTC (permalink / raw) To: maarten.lankhorst Cc: imirkin, linux-kernel, daniel.vetter, robdclark, a.p.zijlstra, mingo Hello, Maarten. Is this patch already queued for 3.12-rcX ? I expect this patch be committed before sending a patch for 3.11-stable. Regards. ---------- >From a1b01c858143c2c2c92b17e7df096042bfe0df6b Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Tue, 24 Sep 2013 23:44:17 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Fix it by explicitly passing a bool which tells whether p != NULL or not. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- kernel/mutex.c | 32 ++++++++++++++++---------------- 1 files changed, 16 insertions(+), 16 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index 6d647ae..d24105b 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -410,7 +410,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, static __always_inline int __sched __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct lockdep_map *nest_lock, unsigned long ip, - struct ww_acquire_ctx *ww_ctx) + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) { struct task_struct *task = current; struct mutex_waiter waiter; @@ -450,7 +450,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -480,7 +480,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -551,7 +551,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -575,7 +575,7 @@ skip_wait: lock_acquired(&lock->dep_map, ip); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); struct mutex_waiter *cur; @@ -615,7 +615,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_nested); @@ -625,7 +625,7 @@ _mutex_lock_nest_lock(struct mutex *lock, struct lockdep_map *nest) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - 0, nest, _RET_IP_, NULL); + 0, nest, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(_mutex_lock_nest_lock); @@ -635,7 +635,7 @@ mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_KILLABLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_killable_nested); @@ -644,7 +644,7 @@ mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested); @@ -682,7 +682,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -697,7 +697,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -809,28 +809,28 @@ __mutex_lock_slowpath(atomic_t *lock_count) struct mutex *lock = container_of(lock_count, struct mutex, count); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_killable_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_KILLABLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_interruptible_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __ww_mutex_lock_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } static noinline int __sched @@ -838,7 +838,7 @@ __ww_mutex_lock_interruptible_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } #endif -- 1.7.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel. 2013-09-09 11:56 ` Tetsuo Handa 2013-09-09 12:07 ` Maarten Lankhorst @ 2013-09-09 12:58 ` Peter Zijlstra 2013-10-03 14:02 ` [PATCH for 3.12-rcX] mutex: Avoid gcc version dependent __builtin_constant_p() usage Tetsuo Handa 1 sibling, 1 reply; 15+ messages in thread From: Peter Zijlstra @ 2013-09-09 12:58 UTC (permalink / raw) To: Tetsuo Handa Cc: maarten.lankhorst, imirkin, linux-kernel, daniel.vetter, robdclark, mingo On Mon, Sep 09, 2013 at 08:56:53PM +0900, Tetsuo Handa wrote: > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Mon, 9 Sep 2013 20:48:13 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Fix it by explicitly passing a bool which tells whether p != NULL or not. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: <stable@kernel.org> [3.11+] > --- > kernel/mutex.c | 32 ++++++++++++++++---------------- > 1 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/kernel/mutex.c b/kernel/mutex.c > index a52ee7bb..a2b80f1 100644 > --- a/kernel/mutex.c > +++ b/kernel/mutex.c > @@ -408,7 +408,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, > static __always_inline int __sched > __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct lockdep_map *nest_lock, unsigned long ip, > - struct ww_acquire_ctx *ww_ctx) > + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) > { > struct task_struct *task = current; > struct mutex_waiter waiter; > @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct task_struct *owner; > struct mspin_node node; > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > if ((atomic_read(&lock->count) == 1) && > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > lock_acquired(&lock->dep_map, ip); > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww; > ww = container_of(lock, struct ww_mutex, base); > > @@ -548,7 +548,7 @@ slowpath: > goto err; > } > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > ret = __mutex_lock_check_stamp(lock, ww_ctx); > if (ret) > goto err; > @@ -568,7 +568,7 @@ done: > mutex_remove_waiter(lock, &waiter, current_thread_info()); > mutex_set_owner(lock); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww = container_of(lock, > struct ww_mutex, > base); > @@ -618,7 +618,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) > { > might_sleep(); > __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, > - subclass, NULL, _RET_IP_, NULL); > + subclass, NULL, _RET_IP_, NULL, 0); > } > > EXPORT_SYMBOL_GPL(mutex_lock_nested); This is a sad patch, but provided it actually generates similar code I suppose its the best we can do bar whole sale deprecating gcc-3. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH for 3.12-rcX] mutex: Avoid gcc version dependent __builtin_constant_p() usage. 2013-09-09 12:58 ` Peter Zijlstra @ 2013-10-03 14:02 ` Tetsuo Handa 2013-10-16 20:20 ` Tetsuo Handa 0 siblings, 1 reply; 15+ messages in thread From: Tetsuo Handa @ 2013-10-03 14:02 UTC (permalink / raw) To: peterz, maarten.lankhorst Cc: imirkin, linux-kernel, daniel.vetter, robdclark, mingo Peter Zijlstra wrote: > On Mon, Sep 09, 2013 at 08:56:53PM +0900, Tetsuo Handa wrote: > > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Date: Mon, 9 Sep 2013 20:48:13 +0900 > > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > > > Fix it by explicitly passing a bool which tells whether p != NULL or not. > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > Cc: <stable@kernel.org> [3.11+] > > --- > > kernel/mutex.c | 32 ++++++++++++++++---------------- > > 1 files changed, 16 insertions(+), 16 deletions(-) > > > > diff --git a/kernel/mutex.c b/kernel/mutex.c > > index a52ee7bb..a2b80f1 100644 > > --- a/kernel/mutex.c > > +++ b/kernel/mutex.c > > @@ -408,7 +408,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, > > static __always_inline int __sched > > __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > > struct lockdep_map *nest_lock, unsigned long ip, > > - struct ww_acquire_ctx *ww_ctx) > > + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) > > { > > struct task_struct *task = current; > > struct mutex_waiter waiter; > > @@ -448,7 +448,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > > struct task_struct *owner; > > struct mspin_node node; > > > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > > + if (use_ww_ctx && ww_ctx->acquired > 0) { > > struct ww_mutex *ww; > > > > ww = container_of(lock, struct ww_mutex, base); > > @@ -478,7 +478,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > > if ((atomic_read(&lock->count) == 1) && > > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > > lock_acquired(&lock->dep_map, ip); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > > + if (use_ww_ctx) { > > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > > > > @@ -548,7 +548,7 @@ slowpath: > > goto err; > > } > > > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > > + if (use_ww_ctx && ww_ctx->acquired > 0) { > > ret = __mutex_lock_check_stamp(lock, ww_ctx); > > if (ret) > > goto err; > > @@ -568,7 +568,7 @@ done: > > mutex_remove_waiter(lock, &waiter, current_thread_info()); > > mutex_set_owner(lock); > > > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > > + if (use_ww_ctx) { > > struct ww_mutex *ww = container_of(lock, > > struct ww_mutex, > > base); > > @@ -618,7 +618,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) > > { > > might_sleep(); > > __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, > > - subclass, NULL, _RET_IP_, NULL); > > + subclass, NULL, _RET_IP_, NULL, 0); > > } > > > > EXPORT_SYMBOL_GPL(mutex_lock_nested); > > This is a sad patch, but provided it actually generates similar code I > suppose its the best we can do bar whole sale deprecating gcc-3. > Can the patch below go to 3.12-rcX (and the patch above to 3.11-stable which does the same thing)? Regards. ---------- >From a1b01c858143c2c2c92b17e7df096042bfe0df6b Mon Sep 17 00:00:00 2001 From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Tue, 24 Sep 2013 23:44:17 +0900 Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. Commit 040a0a37 "mutex: Add support for wound/wait style locks" used "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. Fix it by explicitly passing a bool which tells whether p != NULL or not. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- kernel/mutex.c | 32 ++++++++++++++++---------------- 1 files changed, 16 insertions(+), 16 deletions(-) diff --git a/kernel/mutex.c b/kernel/mutex.c index 6d647ae..d24105b 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -410,7 +410,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, static __always_inline int __sched __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct lockdep_map *nest_lock, unsigned long ip, - struct ww_acquire_ctx *ww_ctx) + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) { struct task_struct *task = current; struct mutex_waiter waiter; @@ -450,7 +450,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, struct task_struct *owner; struct mspin_node node; - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -480,7 +480,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, if ((atomic_read(&lock->count) == 1) && (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { lock_acquired(&lock->dep_map, ip); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww; ww = container_of(lock, struct ww_mutex, base); @@ -551,7 +551,7 @@ slowpath: goto err; } - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { + if (use_ww_ctx && ww_ctx->acquired > 0) { ret = __mutex_lock_check_stamp(lock, ww_ctx); if (ret) goto err; @@ -575,7 +575,7 @@ skip_wait: lock_acquired(&lock->dep_map, ip); mutex_set_owner(lock); - if (!__builtin_constant_p(ww_ctx == NULL)) { + if (use_ww_ctx) { struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); struct mutex_waiter *cur; @@ -615,7 +615,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_nested); @@ -625,7 +625,7 @@ _mutex_lock_nest_lock(struct mutex *lock, struct lockdep_map *nest) { might_sleep(); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, - 0, nest, _RET_IP_, NULL); + 0, nest, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(_mutex_lock_nest_lock); @@ -635,7 +635,7 @@ mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_KILLABLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_killable_nested); @@ -644,7 +644,7 @@ mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass) { might_sleep(); return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, - subclass, NULL, _RET_IP_, NULL); + subclass, NULL, _RET_IP_, NULL, 0); } EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested); @@ -682,7 +682,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -697,7 +697,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) might_sleep(); ret = __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, - 0, &ctx->dep_map, _RET_IP_, ctx); + 0, &ctx->dep_map, _RET_IP_, ctx, 1); if (!ret && ctx->acquired > 1) return ww_mutex_deadlock_injection(lock, ctx); @@ -809,28 +809,28 @@ __mutex_lock_slowpath(atomic_t *lock_count) struct mutex *lock = container_of(lock_count, struct mutex, count); __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_killable_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_KILLABLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __mutex_lock_interruptible_slowpath(struct mutex *lock) { return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, NULL); + NULL, _RET_IP_, NULL, 0); } static noinline int __sched __ww_mutex_lock_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } static noinline int __sched @@ -838,7 +838,7 @@ __ww_mutex_lock_interruptible_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { return __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, 0, - NULL, _RET_IP_, ctx); + NULL, _RET_IP_, ctx, 1); } #endif -- 1.7.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH for 3.12-rcX] mutex: Avoid gcc version dependent __builtin_constant_p() usage. 2013-10-03 14:02 ` [PATCH for 3.12-rcX] mutex: Avoid gcc version dependent __builtin_constant_p() usage Tetsuo Handa @ 2013-10-16 20:20 ` Tetsuo Handa 0 siblings, 0 replies; 15+ messages in thread From: Tetsuo Handa @ 2013-10-16 20:20 UTC (permalink / raw) To: mingo; +Cc: linux-kernel, torvalds I'm worrying that this patch becomes too late for backporting to 3.11-stable. Since there is no objection, can this patch go? Peter Zijlstra wrote: > This is a sad patch, but provided it actually generates similar code I > suppose its the best we can do bar whole sale deprecating gcc-3. Tetsuo Handa wrote: > Can the patch below go to 3.12-rcX (and the patch above to 3.11-stable which > does the same thing)? > > Regards. > ---------- > >From a1b01c858143c2c2c92b17e7df096042bfe0df6b Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Date: Tue, 24 Sep 2013 23:44:17 +0900 > Subject: [PATCH] mutex: Avoid gcc version dependent __builtin_constant_p() usage. > > Commit 040a0a37 "mutex: Add support for wound/wait style locks" used > "!__builtin_constant_p(p == NULL)" but gcc 3.x cannot handle such expression > correctly, leading to boot failure when built with CONFIG_DEBUG_MUTEXES=y. > > Fix it by explicitly passing a bool which tells whether p != NULL or not. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > kernel/mutex.c | 32 ++++++++++++++++---------------- > 1 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/kernel/mutex.c b/kernel/mutex.c > index 6d647ae..d24105b 100644 > --- a/kernel/mutex.c > +++ b/kernel/mutex.c > @@ -410,7 +410,7 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, > static __always_inline int __sched > __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct lockdep_map *nest_lock, unsigned long ip, > - struct ww_acquire_ctx *ww_ctx) > + struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx) > { > struct task_struct *task = current; > struct mutex_waiter waiter; > @@ -450,7 +450,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > struct task_struct *owner; > struct mspin_node node; > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > struct ww_mutex *ww; > > ww = container_of(lock, struct ww_mutex, base); > @@ -480,7 +480,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, > if ((atomic_read(&lock->count) == 1) && > (atomic_cmpxchg(&lock->count, 1, 0) == 1)) { > lock_acquired(&lock->dep_map, ip); > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww; > ww = container_of(lock, struct ww_mutex, base); > > @@ -551,7 +551,7 @@ slowpath: > goto err; > } > > - if (!__builtin_constant_p(ww_ctx == NULL) && ww_ctx->acquired > 0) { > + if (use_ww_ctx && ww_ctx->acquired > 0) { > ret = __mutex_lock_check_stamp(lock, ww_ctx); > if (ret) > goto err; > @@ -575,7 +575,7 @@ skip_wait: > lock_acquired(&lock->dep_map, ip); > mutex_set_owner(lock); > > - if (!__builtin_constant_p(ww_ctx == NULL)) { > + if (use_ww_ctx) { > struct ww_mutex *ww = container_of(lock, struct ww_mutex, base); > struct mutex_waiter *cur; > > @@ -615,7 +615,7 @@ mutex_lock_nested(struct mutex *lock, unsigned int subclass) > { > might_sleep(); > __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, > - subclass, NULL, _RET_IP_, NULL); > + subclass, NULL, _RET_IP_, NULL, 0); > } > > EXPORT_SYMBOL_GPL(mutex_lock_nested); > @@ -625,7 +625,7 @@ _mutex_lock_nest_lock(struct mutex *lock, struct lockdep_map *nest) > { > might_sleep(); > __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, > - 0, nest, _RET_IP_, NULL); > + 0, nest, _RET_IP_, NULL, 0); > } > > EXPORT_SYMBOL_GPL(_mutex_lock_nest_lock); > @@ -635,7 +635,7 @@ mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass) > { > might_sleep(); > return __mutex_lock_common(lock, TASK_KILLABLE, > - subclass, NULL, _RET_IP_, NULL); > + subclass, NULL, _RET_IP_, NULL, 0); > } > EXPORT_SYMBOL_GPL(mutex_lock_killable_nested); > > @@ -644,7 +644,7 @@ mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass) > { > might_sleep(); > return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, > - subclass, NULL, _RET_IP_, NULL); > + subclass, NULL, _RET_IP_, NULL, 0); > } > > EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested); > @@ -682,7 +682,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) > > might_sleep(); > ret = __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, > - 0, &ctx->dep_map, _RET_IP_, ctx); > + 0, &ctx->dep_map, _RET_IP_, ctx, 1); > if (!ret && ctx->acquired > 1) > return ww_mutex_deadlock_injection(lock, ctx); > > @@ -697,7 +697,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) > > might_sleep(); > ret = __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, > - 0, &ctx->dep_map, _RET_IP_, ctx); > + 0, &ctx->dep_map, _RET_IP_, ctx, 1); > > if (!ret && ctx->acquired > 1) > return ww_mutex_deadlock_injection(lock, ctx); > @@ -809,28 +809,28 @@ __mutex_lock_slowpath(atomic_t *lock_count) > struct mutex *lock = container_of(lock_count, struct mutex, count); > > __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, > - NULL, _RET_IP_, NULL); > + NULL, _RET_IP_, NULL, 0); > } > > static noinline int __sched > __mutex_lock_killable_slowpath(struct mutex *lock) > { > return __mutex_lock_common(lock, TASK_KILLABLE, 0, > - NULL, _RET_IP_, NULL); > + NULL, _RET_IP_, NULL, 0); > } > > static noinline int __sched > __mutex_lock_interruptible_slowpath(struct mutex *lock) > { > return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, > - NULL, _RET_IP_, NULL); > + NULL, _RET_IP_, NULL, 0); > } > > static noinline int __sched > __ww_mutex_lock_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) > { > return __mutex_lock_common(&lock->base, TASK_UNINTERRUPTIBLE, 0, > - NULL, _RET_IP_, ctx); > + NULL, _RET_IP_, ctx, 1); > } > > static noinline int __sched > @@ -838,7 +838,7 @@ __ww_mutex_lock_interruptible_slowpath(struct ww_mutex *lock, > struct ww_acquire_ctx *ctx) > { > return __mutex_lock_common(&lock->base, TASK_INTERRUPTIBLE, 0, > - NULL, _RET_IP_, ctx); > + NULL, _RET_IP_, ctx, 1); > } > > #endif > -- > 1.7.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-10-16 20:20 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-07 16:00 [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootable kernel Tetsuo Handa 2013-09-08 4:32 ` Tetsuo Handa 2013-09-08 5:28 ` Ilia Mirkin 2013-09-08 7:24 ` Tetsuo Handa 2013-09-08 7:42 ` Ilia Mirkin 2013-09-08 8:11 ` Maarten Lankhorst 2013-09-08 11:53 ` [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel Tetsuo Handa 2013-09-08 20:36 ` Maarten Lankhorst 2013-09-09 11:56 ` Tetsuo Handa 2013-09-09 12:07 ` Maarten Lankhorst 2013-09-09 13:22 ` Tetsuo Handa 2013-09-24 14:51 ` Tetsuo Handa 2013-09-09 12:58 ` Peter Zijlstra 2013-10-03 14:02 ` [PATCH for 3.12-rcX] mutex: Avoid gcc version dependent __builtin_constant_p() usage Tetsuo Handa 2013-10-16 20:20 ` Tetsuo Handa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox