linux-kernel - Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 26 Oct 2015 22:32:36 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Tejun Heo <tj@...nel.org>, Ingo Molnar <mingo@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Lai Jiangshan <jiangshanlai@...il.com>,
	Dipankar Sarma <dipankar@...ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Josh Triplett <josh@...htriplett.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	David Howells <dhowells@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	Frédéric Weisbecker <fweisbec@...il.com>,
	Oleg Nesterov <oleg@...hat.com>,
	pranith kumar <bobby.prani@...il.com>,
	Patrick Marlier <patrick.marlier@...il.com>
Subject: Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use
 lockless_dereference()

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@...ux.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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/