All of lore.kernel.org
 help / color / mirror / Atom feed
From: Prateek Sood <prsood@codeaurora.org>
To: dbueso@suse.de, andrea.parri@amarulasolutions.com,
	peterz@infradead.org, mingo@redhat.com
Cc: linux-kernel@vger.kernel.org, sramana@codeaurora.org,
	Prateek Sood <prsood@codeaurora.org>
Subject: [PATCH] percpu_rwsem: fix missed wakeup due to reordering of load
Date: Fri, 21 Dec 2018 12:59:13 +0530	[thread overview]
Message-ID: <1545377353-30441-1-git-send-email-prsood@codeaurora.org> (raw)
In-Reply-To: <aa864e194ee4c987b7e2025d1fe6e88b@suse.de>

P1 is releaseing the cpu_hotplug_lock and P2 is acquiring
cpu_hotplug_lock.

P1                                               P2
percpu_up_read() path                      percpu_down_write() path

                                          rcu_sync_enter() //gp_state=GP_PASSED

rcu_sync_is_idle() //returns false        down_write(rw_sem)

__percpu_up_read()

[L] task = rcu_dereference(w->task) //NULL

smp_rmb()                                  [S] w->task = current

                                            smp_mb()

                                           [L] readers_active_check() //fails
					     schedule()

[S] __this_cpu_dec(read_count)

Since load of task can result in NULL, it can lead to missed wakeup
in rcuwait_wake_up(). Above sequence violated the following constraint
in rcuwait_wake_up():

	 WAIT                WAKE
[S] tsk = current	  [S] cond = true
MB (A)	                    MB (B)
[L] cond		  [L] tsk

This can happen as smp_rmb() in rcuwait_wake_up() will provide ordering
of load before barrier with load and store after barrier for arm64
architecture. Here the requirement is to order store before smp_rmb()
with load after the smp_rmb().

For the usage of rcuwait_wake_up() in __percpu_up_read() full barrier
(smp_mb) is required to complete the constraint of rcuwait_wake_up().

Signed-off-by: Prateek Sood <prsood@codeaurora.org>
Acked-by: Davidlohr Bueso <dbueso@suse.de>

---
 kernel/exit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index ac1a814..696e0e1 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -298,7 +298,7 @@ void rcuwait_wake_up(struct rcuwait *w)
 	/*
 	 * Order condition vs @task, such that everything prior to the load
 	 * of @task is visible. This is the condition as to why the user called
-	 * rcuwait_trywake() in the first place. Pairs with set_current_state()
+	 * rcuwait_wake_up() in the first place. Pairs with set_current_state()
 	 * barrier (A) in rcuwait_wait_event().
 	 *
 	 *    WAIT                WAKE
@@ -306,7 +306,7 @@ void rcuwait_wake_up(struct rcuwait *w)
 	 *        MB (A)	      MB (B)
 	 *    [L] cond		  [L] tsk
 	 */
-	smp_rmb(); /* (B) */
+	smp_mb(); /* (B) */
 
 	/*
 	 * Avoid using task_rcu_dereference() magic as long as we are careful,
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


  reply	other threads:[~2018-12-21  7:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-30 15:10 [PATCH] percpu_rwsem: fix missed wakeup due to reordering of load Prateek Sood
2018-12-03  6:38 ` Davidlohr Bueso
2018-12-03 19:36   ` Prateek Sood
2018-12-12 14:26     ` Prateek Sood
2018-12-12 15:31       ` Davidlohr Bueso
2018-12-21  7:29         ` Prateek Sood [this message]
2018-12-21  9:45           ` Andrea Parri
2018-12-12 15:28 ` Andrea Parri
2018-12-21  6:35   ` Prateek Sood
2019-01-21 11:25 ` [tip:locking/core] sched/wait: Fix rcuwait_wake_up() ordering tip-bot for Prateek Sood

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=1545377353-30441-1-git-send-email-prsood@codeaurora.org \
    --to=prsood@codeaurora.org \
    --cc=andrea.parri@amarulasolutions.com \
    --cc=dbueso@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sramana@codeaurora.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 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.