From: Bart Van Assche <bart.vanassche@sandisk.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
"mingo@kernel.org" <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Johannes Weiner" <hannes@cmpxchg.org>,
Neil Brown <neilb@suse.de>, Michael Shaver <jmshaver@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sched: Avoid that __wait_on_bit_lock() hangs
Date: Wed, 3 Aug 2016 14:51:23 -0700 [thread overview]
Message-ID: <17b65ff9-215f-ab74-9f5f-15dbd308d054@sandisk.com> (raw)
In-Reply-To: <20160803213006.GA11712@redhat.com>
On 08/03/2016 02:30 PM, Oleg Nesterov wrote:
> On 08/03, Bart Van Assche wrote:
>> try_to_wake_up() locks task_struct.pi_lock but abort_exclusive_wait() not.
>> My assumption is that the following sequence of events leads to the lockup
>> that I had mentioned in the description of my patch:
>> * try_to_wake_up() is called for the task that will execute
>> abort_exclusive_wait().
>> * After try_to_wake_up() has checked task_struct.state and before
>> autoremove_wake_function() has tried to remove the task from the wait
>> queue, abort_exclusive_wait() is executed for the same task.
>
> But we do not care if we race with another try_to_wake_up(), or even with
> another exclusive wake_up_nr(wq)/whatever unless wq is the same.
>
> And if this wq is the same, then wake_up_nr() will do try_to_wake_up/autoremove
> either before or after abort_exclusive_wait(), wake_up_nr() takes the same
> wq->lock.
>
> And this means that abort_exclusive_wait() can't be called "After try_to_wake_up()"
> and "before autoremove_wake_function()".
Hello Oleg,
It is possible that my analysis is wrong. But what I see is that my
patch makes the lockup disappear. However, what I had not expected is
that I ran into the following (probably caused by the patch at the start
of this thread):
WARNING: CPU: 1 PID: 26023 at lib/list_debug.c:33 __list_add+0x89/0xb0
list_add corruption. prev->next should be next (ffff88047ff4b0c8), but
was ffff8803cbc037e0. (prev=ffff8803c863bd80).
Call Trace:
[<ffffffff81320137>] dump_stack+0x68/0xa1
[<ffffffff81061c46>] __warn+0xc6/0xe0
[<ffffffff81061caa>] warn_slowpath_fmt+0x4a/0x50
[<ffffffff8133d5d9>] __list_add+0x89/0xb0
[<ffffffff810ab5c9>] prepare_to_wait_exclusive+0x79/0x80
[<ffffffff8161fa2f>] __wait_on_bit_lock+0x2f/0xa0
[<ffffffff8114fe89>] __lock_page+0xb9/0xc0
[<ffffffff81165db0>] truncate_inode_pages_range+0x3e0/0x760
[<ffffffff81166140>] truncate_inode_pages+0x10/0x20
[ ... ]
So I started testing the patch below that should fix the same hang but
without triggering any wait list corruption.
Bart.
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index f15d6b6..4e3f651 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -282,7 +282,7 @@ void abort_exclusive_wait(wait_queue_head_t *q,
wait_queue_t *wait,
spin_lock_irqsave(&q->lock, flags);
if (!list_empty(&wait->task_list))
list_del_init(&wait->task_list);
- else if (waitqueue_active(q))
+ if (waitqueue_active(q))
__wake_up_locked_key(q, mode, key);
spin_unlock_irqrestore(&q->lock, flags);
}
next prev parent reply other threads:[~2016-08-03 21:51 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-03 16:35 Bart Van Assche
2016-08-03 18:11 ` Peter Zijlstra
2016-08-03 18:56 ` Bart Van Assche
2016-08-03 21:30 ` Oleg Nesterov
2016-08-03 21:51 ` Bart Van Assche [this message]
2016-08-04 14:09 ` Peter Zijlstra
2016-08-04 14:31 ` Bart Van Assche
2016-08-05 17:41 ` Bart Van Assche
2016-08-08 10:22 ` Peter Zijlstra
2016-08-08 14:38 ` Bart Van Assche
2016-08-08 16:20 ` Oleg Nesterov
2016-08-08 18:31 ` Bart Van Assche
2016-08-09 17:14 ` Oleg Nesterov
2016-08-09 18:48 ` Bart Van Assche
2016-08-09 23:10 ` Bart Van Assche
2016-08-10 10:45 ` Oleg Nesterov
2016-08-10 16:01 ` Bart Van Assche
2016-08-10 16:27 ` Oleg Nesterov
2016-08-10 19:58 ` Bart Van Assche
2016-08-11 17:36 ` Oleg Nesterov
2016-08-12 16:16 ` Oleg Nesterov
2016-08-12 16:27 ` Bart Van Assche
2016-08-12 22:47 ` Bart Van Assche
2016-08-13 16:32 ` Oleg Nesterov
2016-08-15 23:39 ` Bart Van Assche
2016-08-16 13:06 ` Oleg Nesterov
2016-08-16 16:54 ` Bart Van Assche
2016-08-17 17:30 ` Oleg Nesterov
2016-08-13 17:07 ` Oleg Nesterov
2016-08-09 23:56 ` Bart Van Assche
2016-08-10 10:57 ` Oleg Nesterov
2016-08-10 11:03 ` Peter Zijlstra
2016-08-04 0:05 ` Bart Van Assche
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17b65ff9-215f-ab74-9f5f-15dbd308d054@sandisk.com \
--to=bart.vanassche@sandisk.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=jmshaver@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=neilb@suse.de \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®