From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Tejun Heo" <tj@kernel.org>, "Ingo Molnar" <mingo@kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Lai Jiangshan" <jiangshanlai@gmail.com>,
"Dipankar Sarma" <dipankar@in.ibm.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
"Josh Triplett" <josh@joshtriplett.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Peter Zijlstra" <peterz@infradead.org>,
"Steven Rostedt" <rostedt@goodmis.org>,
"David Howells" <dhowells@redhat.com>,
"Eric Dumazet" <edumazet@google.com>,
"Darren Hart" <dvhart@linux.intel.com>,
"Frédéric Weisbecker" <fweisbec@gmail.com>,
"Oleg Nesterov" <oleg@redhat.com>,
"pranith kumar" <bobby.prani@gmail.com>,
"Patrick Marlier" <patrick.marlier@gmail.com>
Subject: Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference()
Date: Mon, 26 Oct 2015 22:32:36 -0700 [thread overview]
Message-ID: <20151027053236.GK5105@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFxHeM2vceDGC7YnDMfN4qu0DYVN=4iRGuC-naj1VnDw_w@mail.gmail.com>
On Tue, Oct 27, 2015 at 12:37:16PM +0900, Linus Torvalds wrote:
> On Mon, Oct 26, 2015 at 11:55 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> >> struct bdi_writeback *last_wb = NULL;
> >> struct bdi_writeback *wb = list_entry_rcu(&bdi->wb_list,
> >
> > I believe that the above should instead be:
> >
> > struct bdi_writeback *wb = list_entry_rcu(bdi->wb_list.next,
>
> I don't think you can do that.
>
> You haven't even taken the RCU read lock yet at this point.
>
> What the code seems to try to do is to get the "head pointer" of the
> list before taking the read lock (since _that_ is stable), and then
> follow the list under the lock.
>
> You're making it actually follow the first RCU pointer too early.
Good point, color me dazed and confused. :-/
Thanx, Paul
> That said, I'm not sure why it doesn't just do the normal
>
> rcu_read_lock();
> list_for_each_entry_rcu(wb, &bdi->wb_list, bdi_node) {
> ....
> }
> rcu_read_unlock();
>
> like the other places do. It looks like it wants that
> "list_for_each_entry_continue_rcu()" because it does that odd "pin
> entry and drop rcu lock and retake it and continue where you left
> off", but I'm not sure why the continue version would be so
> different.. It's going to do that "follow next entry" regardless, and
> the "goto restart" doesn't look like it actually adds anything. If
> following the next pointer is ok even after having released the RCU
> read lock, then I'm not seeing why the end of the loop couldn't just
> do
>
> rcu_read_unlock();
> wb_wait_for_completion(bdi, &fallback_work_done);
> rcu_read_lock();
>
> and just continue the loop (and the pinning of "wb" and releasing the
> "last_wb" thing in the *next* iteration should make it all work the
> same).
>
> Adding Tejun to the cc, because this is his code and there's probably
> something subtle I'm missing. Tejun, can you take a look? It's
> bdi_split_work_to_wbs() in fs/fs-writeback.c.
>
> Linus
>
next prev parent reply other threads:[~2015-10-27 5:32 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-06 16:13 [PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4 Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 01/13] sched: Export sched_setscheduler_nocheck Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 02/13] rcu: Use rcu_callback_t in call_rcu*() and friends Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 03/13] rcu: Use call_rcu_func_t to replace explicit type equivalents Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for Tiny and Tree RCU readers Paul E. McKenney
2015-10-06 16:20 ` [Kernel networking modules.] OSI levels 2 & 3, Assistance - If anyone knows anyone in the US. North West region John D Allen, Leveridge Systems INC
2015-10-06 16:44 ` [PATCH tip/core/rcu 04/13] rcu: Don't disable preemption for Tiny and Tree RCU readers Josh Triplett
2015-10-06 17:01 ` Paul E. McKenney
2015-10-06 17:16 ` Josh Triplett
2015-10-06 17:42 ` Paul E. McKenney
2015-10-06 17:46 ` Josh Triplett
2015-10-06 20:05 ` Peter Zijlstra
2015-10-06 20:18 ` Paul E. McKenney
2015-10-06 20:52 ` Peter Zijlstra
2015-10-06 21:05 ` Paul E. McKenney
2015-10-07 7:19 ` Peter Zijlstra
2015-10-06 16:13 ` [PATCH tip/core/rcu 05/13] rcu: Eliminate panic when silly boot-time fanout specified Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 06/13] rcu: Add online/offline info to stall warning message Paul E. McKenney
2015-10-06 17:15 ` Josh Triplett
2015-10-06 16:13 ` [PATCH tip/core/rcu 07/13] rcu: Move preemption disabling out of __srcu_read_lock() Paul E. McKenney
2015-10-06 17:18 ` Josh Triplett
2015-10-06 17:36 ` Paul E. McKenney
2015-10-06 17:43 ` Josh Triplett
2015-10-06 18:07 ` Paul E. McKenney
2015-10-06 20:07 ` Peter Zijlstra
2015-10-06 20:19 ` Paul E. McKenney
2015-10-06 20:32 ` Peter Zijlstra
2015-10-06 21:03 ` Paul E. McKenney
2015-10-07 7:20 ` Peter Zijlstra
2015-10-07 14:18 ` Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 08/13] rcu: Finish folding ->fqs_state into ->gp_state Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 09/13] rcu: Correct comment for values of ->gp_state field Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 10/13] rcu: Add rcu_pointer_handoff() Paul E. McKenney
2015-10-06 17:21 ` Josh Triplett
2015-10-06 17:31 ` Paul E. McKenney
2015-10-06 17:36 ` Josh Triplett
2015-10-06 20:27 ` Peter Zijlstra
2015-10-06 21:02 ` Paul E. McKenney
2015-10-07 7:22 ` Peter Zijlstra
2015-10-07 14:20 ` Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference() Paul E. McKenney
2015-10-26 8:45 ` Ingo Molnar
2015-10-26 14:55 ` Paul E. McKenney
2015-10-26 18:02 ` Ingo Molnar
2015-10-27 3:37 ` Linus Torvalds
2015-10-27 5:19 ` Tejun Heo
2015-10-27 5:33 ` Paul E. McKenney
2015-10-28 8:33 ` Ingo Molnar
2015-10-28 20:35 ` Patrick Marlier
2015-10-29 0:00 ` Paul E. McKenney
2015-10-29 2:13 ` Tejun Heo
2015-10-28 20:58 ` [tip:core/rcu] fs/writeback, rcu: Don't use list_entry_rcu() for pointer offsetting in bdi_split_work_to_wbs() tip-bot for Tejun Heo
2015-10-27 5:32 ` Paul E. McKenney [this message]
2015-10-06 16:13 ` [PATCH tip/core/rcu 12/13] rcu: Remove deprecated rcu_lockdep_assert() Paul E. McKenney
2015-10-06 16:13 ` [PATCH tip/core/rcu 13/13] rculist: Use WRITE_ONCE() when deleting from reader-visible list Paul E. McKenney
2015-10-06 17:23 ` [PATCH tip/core/rcu 0/13] Miscellaneous fixes for 4.4 Josh Triplett
2015-10-06 17:38 ` Paul E. McKenney
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=20151027053236.GK5105@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=bobby.prani@gmail.com \
--cc=dhowells@redhat.com \
--cc=dipankar@in.ibm.com \
--cc=dvhart@linux.intel.com \
--cc=edumazet@google.com \
--cc=fweisbec@gmail.com \
--cc=jiangshanlai@gmail.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=patrick.marlier@gmail.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.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;
as well as URLs for NNTP newsgroup(s).