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


Groups > linux.kernel > #1256197

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

From Ingo Molnar <mingo@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH tip/core/rcu 11/13] rculist: Make list_entry_rcu() use lockless_dereference()
Date 2015-10-26 19:10 +0100
Message-ID <qnUnw-35z-7@gated-at.bofh.it> (permalink)
References <qgD85-2O1-3@gated-at.bofh.it> <qgD85-2O1-5@gated-at.bofh.it> <qgD87-2O1-61@gated-at.bofh.it> <qnLDz-60G-1@gated-at.bofh.it> <qnRpD-143-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> > It's this new usage in fs/fs-writeback.c:
> > 
> > static void bdi_split_work_to_wbs(struct backing_dev_info *bdi,
> >                                   struct wb_writeback_work *base_work,
> >                                   bool skip_if_busy)
> > {
> >         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,
> 
> After all, RCU read-side list primitives need to fetch pointers in order to 
> traverse those pointers in an RCU-safe manner.  The patch below clears this up 
> for me, does it also work for you?

Are you sure about that?

I considered this solution too, but the code goes like this:

static void bdi_split_work_to_wbs(struct backing_dev_info *bdi,
                                  struct wb_writeback_work *base_work,
                                  bool skip_if_busy)
{
        struct bdi_writeback *last_wb = NULL;
        struct bdi_writeback *wb = list_entry_rcu(&bdi->wb_list,
                                                struct bdi_writeback, bdi_node);

        might_sleep();
restart:
        rcu_read_lock();
        list_for_each_entry_continue_rcu(wb, &bdi->wb_list, bdi_node) {

and list_for_each_entry_continue_rcu() will start the iteration with the next 
entry. So if you initialize the head with .next, then we'll start with 
.next->next, i.e. we skip the first entry.

That seems to change behavior and break the logic.

Another solution I considered is to use bd->wb_list.next->prev, but that, beyond 
being ugly, causes actual extra runtime overhead - for something that seems 
academical.

Thanks,

	Ingo
--
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 | NextPrevious 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