From: Frederic Weisbecker <frederic@kernel.org>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Boqun Feng <boqun.feng@gmail.com>,
linux-kernel@vger.kernel.org, rcu@vger.kernel.org,
linux-doc@vger.kernel.org,
Chen Zhongjin <chenzhongjin@huawei.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
Valentin Schneider <vschneid@redhat.com>,
Neeraj Upadhyay <neeraj.iitr10@gmail.com>,
Joel Fernandes <joel@joelfernandes.org>,
Josh Triplett <josh@joshtriplett.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Zqiang <qiang.zhang1211@gmail.com>,
Kent Overstreet <kent.overstreet@linux.dev>,
Andrew Morton <akpm@linux-foundation.org>,
Heiko Carstens <hca@linux.ibm.com>, Arnd Bergmann <arnd@arndb.de>,
Oleg Nesterov <oleg@redhat.com>,
Christian Brauner <brauner@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Mike Christie <michael.christie@oracle.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Mateusz Guzik <mjguzik@gmail.com>,
Nicholas Piggin <npiggin@gmail.com>,
Peng Zhang <zhangpeng.00@bytedance.com>
Subject: Re: [PATCH 2/2] rcu-tasks: Eliminate deadlocks involving do_exit() and RCU tasks
Date: Thu, 8 Feb 2024 11:43:10 +0100 [thread overview]
Message-ID: <ZcSwPifss+ho3hRt@lothringen> (raw)
In-Reply-To: <cc25e968-6f43-453e-be9e-2851db39218f@paulmck-laptop>
On Thu, Feb 08, 2024 at 01:56:10AM -0800, Paul E. McKenney wrote:
> On Thu, Feb 08, 2024 at 03:10:32AM +0100, Frederic Weisbecker wrote:
> This ordering is not needed. The lock orders addition to this
> list against removal from tasklist. If we hold this lock, either
> the task is already on this list or our holding this lock prevents
> it from removing itself from the tasklist.
>
> We have already scanned the task list, and we have already done
> whatever update we are worried about.
>
> So, if the task was on the tasklist when we scanned, well and
> good. If the task was created after we scanned the tasklist,
> then it cannot possibly access whatever we removed.
>
> But please double-check!!!
Heh, right, another new pattern for me to discover :-/
C r-LOCK
{
}
P0(spinlock_t *LOCK, int *X, int *Y)
{
int r1;
int r2;
r1 = READ_ONCE(*X);
spin_lock(LOCK);
r2 = READ_ONCE(*Y);
spin_unlock(LOCK);
}
P1(spinlock_t *LOCK, int *X, int *Y)
{
spin_lock(LOCK);
WRITE_ONCE(*Y, 1);
spin_unlock(LOCK);
WRITE_ONCE(*X, 1);
}
exists (0:r1=1 /\ 0:r2=0) (* never *)
>
> > > synchronize_rcu_tasks() do_exit()
> > > ---------------------- ---------
> > > //for_each_process_thread()
> > > READ tasklist WRITE rtpcp->rtp_exit_list
> > > LOCK rtpcp->lock UNLOCK rtpcp->lock
> > > smp_mb__after_unlock_lock() WRITE tasklist //unhash_process()
> > > READ rtpcp->rtp_exit_list
> > >
> > > Does this work? Hmm, I'll play with litmus once I have a fresh brain...
>
> First, thank you very much for the review!!!
>
> > ie: does smp_mb__after_unlock_lock() order only what precedes the UNLOCK with
> > the UNLOCK itself? (but then the UNLOCK itself can be reordered with anything
> > that follows)? Or does it also order what follows the UNLOCK with the UNLOCK
> > itself? If both, then it looks ok, otherwise...
>
> If you have this:
>
> earlier_accesses();
> spin_lock(...);
> ill_considered_memory_accesses();
> smp_mb__after_unlock_lock();
> later_accesses();
>
> Then earlier_accesses() will be ordered against later_accesses(), but
> ill_considered_memory_accesses() won't necessarily be ordered. Also,
> any accesses before any prior release of that same lock will be ordered
> against later_accesses().
>
> (In real life, ill_considered_memory_accesses() will be fully ordered
> against either spin_lock() on the one hand or smp_mb__after_unlock_lock()
> on the other, with x86 doing the first and PowerPC doing the second.
> So please try to avoid any ill_considered_memory_accesses().)
Thanks a lot for that explanation!
>
> > Also on the other end, does LOCK/smp_mb__after_unlock_lock() order against what
> > precedes the LOCK? That also is necessary for the above to work.
>
> It looks like an smp_mb__after_spinlock() would also be needed, for
> example, on ARMv8.
>
> > Of course by the time I'm writing this email, litmus would have told me
> > already...
>
> ;-) ;-) ;-)
>
> But I believe that simple locking covers this case. Famous last words...
Indeed, looks right!
Thanks!
> Thanx, Paul
prev parent reply other threads:[~2024-02-08 10:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 22:57 [PATCH 0/2] RCU tasks fixes for v6.9 Boqun Feng
2024-01-29 22:57 ` [PATCH 1/2] rcu-tasks: Repair RCU Tasks Trace quiescence check Boqun Feng
2024-01-29 22:57 ` [PATCH 2/2] rcu-tasks: Eliminate deadlocks involving do_exit() and RCU tasks Boqun Feng
2024-02-07 22:53 ` Frederic Weisbecker
2024-02-08 1:52 ` Frederic Weisbecker
2024-02-08 2:10 ` Frederic Weisbecker
2024-02-08 9:56 ` Paul E. McKenney
2024-02-08 10:43 ` Frederic Weisbecker [this message]
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=ZcSwPifss+ho3hRt@lothringen \
--to=frederic@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=boqun.feng@gmail.com \
--cc=brauner@kernel.org \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=chenzhongjin@huawei.com \
--cc=dietmar.eggemann@arm.com \
--cc=hca@linux.ibm.com \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=juri.lelli@redhat.com \
--cc=kent.overstreet@linux.dev \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mgorman@suse.de \
--cc=michael.christie@oracle.com \
--cc=mingo@redhat.com \
--cc=mjguzik@gmail.com \
--cc=mst@redhat.com \
--cc=neeraj.iitr10@gmail.com \
--cc=npiggin@gmail.com \
--cc=oleg@redhat.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=qiang.zhang1211@gmail.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=surenb@google.com \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=zhangpeng.00@bytedance.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).