* [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
@ 2015-01-20 20:02 Gustavo Bittencourt
2015-01-25 20:55 ` Paul Gortmaker
0 siblings, 1 reply; 7+ messages in thread
From: Gustavo Bittencourt @ 2015-01-20 20:02 UTC (permalink / raw)
To: linux-rt-users
The functions ww_mutex_lock_interruptible and ww_mutex_lock should return -EDEADLK when faced with
a deadlock. To do so, the paramenter detect_deadlock in rt_mutex_slowlock must be TRUE.
This patch corrects potential deadlocks when running PREEMPT_RT with nouveau driver.
Kernel version: 3.14.25-rt22
Signed-off-by: Gustavo Bittencourt <gbitten@gmail.com>
---
kernel/locking/rtmutex.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 6c40660..3f6ef91 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1965,7 +1965,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ww_c
might_sleep();
mutex_acquire(&lock->base.dep_map, 0, 0, _RET_IP_);
- ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 0, ww_ctx);
+ ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 1, ww_ctx);
if (ret)
mutex_release(&lock->base.dep_map, 1, _RET_IP_);
else if (!ret && ww_ctx->acquired > 1)
@@ -1984,7 +1984,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ww_ctx)
mutex_acquire_nest(&lock->base.dep_map, 0, 0, &ww_ctx->dep_map,
_RET_IP_);
- ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 0, ww_ctx);
+ ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 1, ww_ctx);
if (ret)
mutex_release(&lock->base.dep_map, 1, _RET_IP_);
else if (!ret && ww_ctx->acquired > 1)
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-01-20 20:02 [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions Gustavo Bittencourt
@ 2015-01-25 20:55 ` Paul Gortmaker
2015-01-26 15:06 ` Gustavo Bittencourt
0 siblings, 1 reply; 7+ messages in thread
From: Paul Gortmaker @ 2015-01-25 20:55 UTC (permalink / raw)
To: Gustavo Bittencourt; +Cc: linux-rt-users
On Tue, Jan 20, 2015 at 3:02 PM, Gustavo Bittencourt <gbitten@gmail.com> wrote:
> The functions ww_mutex_lock_interruptible and ww_mutex_lock should return -EDEADLK when faced with
> a deadlock. To do so, the paramenter detect_deadlock in rt_mutex_slowlock must be TRUE.
> This patch corrects potential deadlocks when running PREEMPT_RT with nouveau driver.
>
> Kernel version: 3.14.25-rt22
You might want to have a look at this:
http://www.spinics.net/lists/linux-rt-users/msg12259.html
as it also impacts what the deadlock detection flag does.
Paul.
--
>
> Signed-off-by: Gustavo Bittencourt <gbitten@gmail.com>
> ---
> kernel/locking/rtmutex.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 6c40660..3f6ef91 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1965,7 +1965,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ww_c
> might_sleep();
>
> mutex_acquire(&lock->base.dep_map, 0, 0, _RET_IP_);
> - ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 0, ww_ctx);
> + ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 1, ww_ctx);
> if (ret)
> mutex_release(&lock->base.dep_map, 1, _RET_IP_);
> else if (!ret && ww_ctx->acquired > 1)
> @@ -1984,7 +1984,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ww_ctx)
>
> mutex_acquire_nest(&lock->base.dep_map, 0, 0, &ww_ctx->dep_map,
> _RET_IP_);
> - ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 0, ww_ctx);
> + ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 1, ww_ctx);
> if (ret)
> mutex_release(&lock->base.dep_map, 1, _RET_IP_);
> else if (!ret && ww_ctx->acquired > 1)
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-01-25 20:55 ` Paul Gortmaker
@ 2015-01-26 15:06 ` Gustavo Bittencourt
2015-02-17 17:46 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 7+ messages in thread
From: Gustavo Bittencourt @ 2015-01-26 15:06 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: linux-rt-users
I'm not sure if those patches are connected, so I am posting my findings
for anyone.
ABOUT NOUVEAU AND WW_MUTEX:
The nouveau driver intensely uses the ww_mutex. According the
ww-mutex-design.txt documentation, the ww_mutex makes the following
steps when try to lock a resource:
1) It calls the ww_mutex_lock (or ww_mutex_lock_interruptible) function
for normal lock acquisition.
2) If it returns -EDEADLK, the wounded task must drop all already
acquired locks and call the ww_mutex_lock_slow (or
ww_mutex_lock_slow_interruptible) function.
In the nouveau driver, those steps happen in the validate_init function
(drivers/gpu/drm/nouveau_gem.c) when it calls the functions
ttm_bo_reserve (step 1) and ttm_bo_reserve_slowpath (step 2):
validate_init(...)
{
...
ret = ttm_bo_reserve(&nvbo->bo, true, false, true, &op->ticket);
if (ret) {
validate_fini_no_ticket(op, NULL);
if (unlikely(ret == -EDEADLK)) {
ret = ttm_bo_reserve_slowpath(&nvbo->bo, true,
&op->ticket);
...
}
THE PROBLEM:
When I run the PREEMPT_RT in my computer, it freezes as soon I start
some big graphical application (e.g. Firefox or Chrome). The following
execution path happens until reaching an infinite loop:
a) nouveau_gem_ioctl_pushbuf()
b) nouveau_gem_pushbuf_validate()
c) validate_init()
d) ttm_bo_reserve()
e) ttm_bo_reserve_nolru()
f) ww_mutex_lock_interruptible()
g) __ww_mutex_lock_interruptible()
h) rt_mutex_slowlock()
i) rt_mutex_handle_deadlock() -> infinite loop
The infinite loop that I mentioned above is the while(1){...} inside the
rt_mutex_handle_deadlock function:
static void rt_mutex_handle_deadlock(int res, int detect_deadlock,
struct rt_mutex_waiter *w)
{
/*
* If the result is not -EDEADLOCK or the caller requested
* deadlock detection, nothing to do here.
*/
if (res != -EDEADLOCK || detect_deadlock)
return;
/*
* Yell lowdly and stop the task right here.
*/
rt_mutex_print_deadlock(w);
while (1) {
set_current_state(TASK_INTERRUPTIBLE);
schedule();
}
}
CONCLUSION:
My conclusion was the detect_deadlock should be enabled to the
rt_mutex_handle_deadlock function returns -EDEADLK and hence the
ww_mutex_lock_interruptible returns -EDEADLK too. BTW, since I have
applied my patch, I've not had anymore freezing issue.
PS: I am a kernelnewbie so it is possible that I am writing some BS here.
On 01/25/2015 06:55 PM, Paul Gortmaker wrote:
> On Tue, Jan 20, 2015 at 3:02 PM, Gustavo Bittencourt <gbitten@gmail.com> wrote:
>> The functions ww_mutex_lock_interruptible and ww_mutex_lock should return -EDEADLK when faced with
>> a deadlock. To do so, the paramenter detect_deadlock in rt_mutex_slowlock must be TRUE.
>> This patch corrects potential deadlocks when running PREEMPT_RT with nouveau driver.
>>
>> Kernel version: 3.14.25-rt22
>
> You might want to have a look at this:
>
> http://www.spinics.net/lists/linux-rt-users/msg12259.html
>
> as it also impacts what the deadlock detection flag does.
>
> Paul.
> --
>
>>
>> Signed-off-by: Gustavo Bittencourt <gbitten@gmail.com>
>> ---
>> kernel/locking/rtmutex.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
>> index 6c40660..3f6ef91 100644
>> --- a/kernel/locking/rtmutex.c
>> +++ b/kernel/locking/rtmutex.c
>> @@ -1965,7 +1965,7 @@ __ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ww_c
>> might_sleep();
>>
>> mutex_acquire(&lock->base.dep_map, 0, 0, _RET_IP_);
>> - ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 0, ww_ctx);
>> + ret = rt_mutex_slowlock(&lock->base.lock, TASK_INTERRUPTIBLE, NULL, 1, ww_ctx);
>> if (ret)
>> mutex_release(&lock->base.dep_map, 1, _RET_IP_);
>> else if (!ret && ww_ctx->acquired > 1)
>> @@ -1984,7 +1984,7 @@ __ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ww_ctx)
>>
>> mutex_acquire_nest(&lock->base.dep_map, 0, 0, &ww_ctx->dep_map,
>> _RET_IP_);
>> - ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 0, ww_ctx);
>> + ret = rt_mutex_slowlock(&lock->base.lock, TASK_UNINTERRUPTIBLE, NULL, 1, ww_ctx);
>> if (ret)
>> mutex_release(&lock->base.dep_map, 1, _RET_IP_);
>> else if (!ret && ww_ctx->acquired > 1)
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-01-26 15:06 ` Gustavo Bittencourt
@ 2015-02-17 17:46 ` Sebastian Andrzej Siewior
2015-02-18 0:18 ` Gustavo Bittencourt
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-02-17 17:46 UTC (permalink / raw)
To: Gustavo Bittencourt; +Cc: Paul Gortmaker, linux-rt-users
* Gustavo Bittencourt | 2015-01-26 13:06:35 [-0200]:
>I'm not sure if those patches are connected, so I am posting my
>findings for anyone.
could please re-test again v3.18-RT and tell if this patch is still
required or not?
Sebastian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-02-17 17:46 ` Sebastian Andrzej Siewior
@ 2015-02-18 0:18 ` Gustavo Bittencourt
2015-02-18 9:43 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 7+ messages in thread
From: Gustavo Bittencourt @ 2015-02-18 0:18 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: Paul Gortmaker, linux-rt-users
On 02/17/2015 03:46 PM, Sebastian Andrzej Siewior wrote:
> * Gustavo Bittencourt | 2015-01-26 13:06:35 [-0200]:
>
>> I'm not sure if those patches are connected, so I am posting my
>> findings for anyone.
> could please re-test again v3.18-RT and tell if this patch is still
> required or not?
>
> Sebastian
It is still required.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-02-18 0:18 ` Gustavo Bittencourt
@ 2015-02-18 9:43 ` Sebastian Andrzej Siewior
2015-02-23 16:31 ` Gustavo Bittencourt
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2015-02-18 9:43 UTC (permalink / raw)
To: Gustavo Bittencourt; +Cc: Paul Gortmaker, linux-rt-users
* Gustavo Bittencourt | 2015-02-17 22:18:53 [-0200]:
>It is still required.
Applied. Please let me know how that nouveau driver behaves. Last thing
I know is that it randomly explodes.
Sebastian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions
2015-02-18 9:43 ` Sebastian Andrzej Siewior
@ 2015-02-23 16:31 ` Gustavo Bittencourt
0 siblings, 0 replies; 7+ messages in thread
From: Gustavo Bittencourt @ 2015-02-23 16:31 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: Paul Gortmaker, linux-rt-users
On Wed, Feb 18, 2015 at 7:43 AM, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> * Gustavo Bittencourt | 2015-02-17 22:18:53 [-0200]:
>
>>It is still required.
> Applied. Please let me know how that nouveau driver behaves. Last thing
> I know is that it randomly explodes.
>
3.18.7-rt2 is working smoothly.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-02-23 16:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-20 20:02 [PATCH] rtmutex: enable deadlock detection in ww_mutex_lock functions Gustavo Bittencourt
2015-01-25 20:55 ` Paul Gortmaker
2015-01-26 15:06 ` Gustavo Bittencourt
2015-02-17 17:46 ` Sebastian Andrzej Siewior
2015-02-18 0:18 ` Gustavo Bittencourt
2015-02-18 9:43 ` Sebastian Andrzej Siewior
2015-02-23 16:31 ` Gustavo Bittencourt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.