From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753743AbcHZM44 (ORCPT ); Fri, 26 Aug 2016 08:56:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55010 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753512AbcHZM4y (ORCPT ); Fri, 26 Aug 2016 08:56:54 -0400 Date: Fri, 26 Aug 2016 14:45:52 +0200 From: Oleg Nesterov To: Ingo Molnar , Peter Zijlstra Cc: Al Viro , Bart Van Assche , Johannes Weiner , Linus Torvalds , Neil Brown , linux-kernel@vger.kernel.org Subject: [PATCH 2/2] sched/wait: avoid abort_exclusive_wait() in __wait_on_bit_lock() Message-ID: <20160826124552.GB28904@redhat.com> References: <20160826124453.GA28894@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160826124453.GA28894@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 26 Aug 2016 12:45:56 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I think it would be nice to kill abort_exclusive_wait(). This patch changes __wait_on_bit_lock(), but we can do a similar change to remove it from ___wait_event(). We do not need anything tricky to avoid the race, we can just call finish_wait() if action() fails. test_and_set_bit() implies mb() so the lockless list_empty_careful() case is fine, we can not miss the condition if we race with unlock_page(). I am not sure we even want to conditionalize both finish_wait()'s, we could simply call it unconditionally and once before test_and_set(), the spurious wakeup is unlikely case. Signed-off-by: Oleg Nesterov --- kernel/sched/wait.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c index 2bbba01..c10e904 100644 --- a/kernel/sched/wait.c +++ b/kernel/sched/wait.c @@ -423,20 +423,28 @@ int __sched __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q, wait_bit_action_f *action, unsigned mode) { - do { - int ret; + int ret = 0; + for (;;) { prepare_to_wait_exclusive(wq, &q->wait, mode); - if (!test_bit(q->key.bit_nr, q->key.flags)) - continue; - ret = action(&q->key, mode); - if (!ret) - continue; - abort_exclusive_wait(wq, &q->wait, &q->key); - return ret; - } while (test_and_set_bit(q->key.bit_nr, q->key.flags)); - finish_wait(wq, &q->wait); - return 0; + if (test_bit(q->key.bit_nr, q->key.flags)) { + ret = action(&q->key, mode); + /* + * Ensure that clear_bit() + wake_up() right after + * test_and_set_bit() below can't see us; it should + * wake up another exclusive waiter if we fail. + */ + if (ret) + finish_wait(wq, &q->wait); + } + if (!test_and_set_bit(q->key.bit_nr, q->key.flags)) { + if (!ret) + finish_wait(wq, &q->wait); + return 0; + } else if (ret) { + return ret; + } + } } EXPORT_SYMBOL(__wait_on_bit_lock); -- 2.5.0