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 > <paul...@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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/