[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <522CDFC8.9020903@canonical.com>
Date: Sun, 08 Sep 2013 22:36:24 +0200
From: Maarten Lankhorst <maarten.lankhorst@...onical.com>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
CC: imirkin@...m.mit.edu, linux-kernel@...r.kernel.org,
daniel.vetter@...ll.ch, robdclark@...il.com,
a.p.zijlstra@...llo.nl, mingo@...nel.org
Subject: Re: [3.11-rc1] CONFIG_DEBUG_MUTEXES=y using gcc 3.x makes unbootablekernel.
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@...ove.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@...ove.SAKURA.ne.jp>
> Cc: <stable@...nel.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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists