mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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);
  }

  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®