Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1256445

Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference()

Show all headers | View raw

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.

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Ingo Molnar <mingo@kernel.org> - 2015-10-26 09:50 +0100
  Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-26 16:00 +0100
    Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Ingo Molnar <mingo@kernel.org> - 2015-10-26 19:10 +0100
    Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference() Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-27 04:40 +0100
      Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Tejun Heo <tj@kernel.org> - 2015-10-27 06:20 +0100
        Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-27 06:40 +0100
        Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Ingo Molnar <mingo@kernel.org> - 2015-10-28 09:40 +0100
          Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Patrick Marlier <patrick.marlier@gmail.com> - 2015-10-28 21:40 +0100
            Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-29 01:10 +0100
              Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() Tejun Heo <tj@kernel.org> - 2015-10-29 03:20 +0100
        [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 <tipbot@zytor.com> - 2015-10-28 22:00 +0100
      Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use  lockless_dereference() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-27 06:40 +0100

csiph-web