[lttng-dev] [RFC] adding into middle of RCU list

[lttng-dev] [RFC] adding into middle of RCU list

[Paul E. McKenney]

On Fri, Aug 23, 2013 at 02:08:22PM -0700, Paul E. McKenney wrote:
> On Fri, Aug 23, 2013 at 01:16:53PM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > > On Thu, Aug 22, 2013 at 09:33:18PM -0700, Stephen Hemminger wrote:

[ . . . ]

> > > > +
> > > > +/**
> > > > + * Splice an RCU-protected list into an existing list.
> > > > + *
> > > > + * Note that this function blocks in synchronize_rcu()
> > > > + *
> > > > + * Important note: this function is not called concurrently
> > > > + *       with other updates to the list.
> > > > + */
> > > > +static inline void caa_list_splice_init_rcu(struct cds_list_head *list,
> > > > +					    struct cds_list_head *head)
> > > > +{
> > > > +	struct cds_list_head *first = list->next;
> > > > +	struct cds_list_head *last = list->prev;
> > > > +	struct cds_list_head *at = head->next;
> > > > +
> > > > +	if (cds_list_empty(list))
> > > > +		return;
> > > > +
> > > > +	/* "first" and "last" tracking list, so initialize it. */
> > > > +	CDS_INIT_LIST_HEAD(list);
> > > 
> > > This change is happening in the presence of readers on the list, right?
> > > For this to work reliably in the presence of mischievous compilers,
> > > wouldn't CDS_INIT_LIST_HEAD() need to use CMM_ACCESS_ONCE() for its
> > > pointer accesses?
> > 
> > Actually, we have rcu_assign_pointer()/rcu_set_pointer() exactly for
> > this. They even skip the memory barrier if they store a NULL pointer.
> > 
> > > Hmmm...  The kernel version seems to have the same issue...
> > 
> > The compiler memory model of the Linux kernel AFAIK does not require an
> > ACCESS_ONCE() for stores to word-aligned, word-sized integers/pointers,
> > even if those are expected to be read concurrently. For reference, see:
> > 
> > #define __rcu_assign_pointer(p, v, space) \
> >         do { \
> >                 smp_wmb(); \
> >                 (p) = (typeof(*v) __force space *)(v); \
> >         } while (0)
> 
> Or I need to fix this one as well.  ;-)

In that vein...  Is there anything like typeof() that also preserves
sparse's notion of address space?  Wrapping an ACCESS_ONCE() around
"p" in the assignment above results in sparse errors.

							Thanx, Paul

							Thanx, Paul

[lttng-dev] [RFC] adding into middle of RCU list

[Josh Triplett]

On Thu, Aug 29, 2013 at 05:57:33PM -0700, Paul E. McKenney wrote:
> On Fri, Aug 23, 2013 at 02:08:22PM -0700, Paul E. McKenney wrote:
> > On Fri, Aug 23, 2013 at 01:16:53PM -0400, Mathieu Desnoyers wrote:
> > > #define __rcu_assign_pointer(p, v, space) \
> > >         do { \
> > >                 smp_wmb(); \
> > >                 (p) = (typeof(*v) __force space *)(v); \
> > >         } while (0)
> > 
> > Or I need to fix this one as well.  ;-)
> 
> In that vein...  Is there anything like typeof() that also preserves
> sparse's notion of address space?  Wrapping an ACCESS_ONCE() around
> "p" in the assignment above results in sparse errors.

typeof() will preserve sparse's notion of address space as long as you
do typeof(p), not typeof(*p):

$ cat test.c
#define as(n) __attribute__((address_space(n),noderef))
#define __force __attribute__((force))

int main(void)
{
    int target = 0;
    int as(1) *foo = (__force typeof(target) as(1) *) ⌖
    typeof(foo) bar = foo;
    return *bar;
}
$ sparse test.c
test.c:9:13: warning: dereference of noderef expression

Notice that sparse didn't warn on the assignment of foo to bar (because
typeof propagated the address space of 1), and warned on the dereference
of bar (because typeof propagated noderef).

- Josh Triplett

[lttng-dev] [RFC] adding into middle of RCU list

[Paul E. McKenney]

On Thu, Aug 29, 2013 at 07:16:37PM -0700, Josh Triplett wrote:
> On Thu, Aug 29, 2013 at 05:57:33PM -0700, Paul E. McKenney wrote:
> > On Fri, Aug 23, 2013 at 02:08:22PM -0700, Paul E. McKenney wrote:
> > > On Fri, Aug 23, 2013 at 01:16:53PM -0400, Mathieu Desnoyers wrote:
> > > > #define __rcu_assign_pointer(p, v, space) \
> > > >         do { \
> > > >                 smp_wmb(); \
> > > >                 (p) = (typeof(*v) __force space *)(v); \
> > > >         } while (0)
> > > 
> > > Or I need to fix this one as well.  ;-)
> > 
> > In that vein...  Is there anything like typeof() that also preserves
> > sparse's notion of address space?  Wrapping an ACCESS_ONCE() around
> > "p" in the assignment above results in sparse errors.
> 
> typeof() will preserve sparse's notion of address space as long as you
> do typeof(p), not typeof(*p):
> 
> $ cat test.c
> #define as(n) __attribute__((address_space(n),noderef))
> #define __force __attribute__((force))
> 
> int main(void)
> {
>     int target = 0;
>     int as(1) *foo = (__force typeof(target) as(1) *) ⌖
>     typeof(foo) bar = foo;
>     return *bar;
> }
> $ sparse test.c
> test.c:9:13: warning: dereference of noderef expression
> 
> Notice that sparse didn't warn on the assignment of foo to bar (because
> typeof propagated the address space of 1), and warned on the dereference
> of bar (because typeof propagated noderef).

Thank you for the info!

Suppose that I want to do something like this:

#define __rcu_assign_pointer(p, v, space) \
        do { \
                smp_wmb(); \
                ACCESS_ONCE(p) = (typeof(*v) __force space *)(v); \
        } while (0)

Now, this does typeof(*p), so as you noted above sparse complains about
address-space mismatches.  Thus far, I haven't been able to come up with
something that (1) does sparse address-space checking, (2) does C type
checking, and (3) forces the assignment to be volatile.

Any thoughts on how to do this?

							Thanx, Paul