Re: Using sparse to catch invalid RCU dereferences?

On Thu, 2008-04-10 at 15:32 -0700, Paul E. McKenney wrote:
On Wed, Apr 09, 2008 at 10:09:46PM +0200, Johannes Berg wrote:

It might be. There are a number of places where it is legal to access
RCU-protected pointers directly, and all of these would need to be
changed. For example, in the example above, one could do:

foo = NULL;

Ok, that I understand, but sparse always treats NULL specially anyway.

But "int foo = 0;" would need the memory barrier -- index 0 of some
RCU-protected array.

Oh. Hmm, I guess that wouldn't really be possible to find at least not
with sparse right now. Though maybe we can add some sort of annotation
that that special type can't even take zero directly.

You are right -- I was confused. The case where you can omit the
rcu_assign_pointer() would be when building a multiple-element data
structure that is then published as a unit. For example:

p = kmalloc(sizeof(*p), GFP_KERNEL);
q = kmalloc(sizeof(*p), GFP_KERNEL);
p->next = q; /* don't need rcu_assign_pointer() here. */
q->next = NULL; /* or here. */
/* initialize other fields of p and q. */
rcu_assign_pointer(global_pointer, p);

The assignment to p->next doesn't have to be rcu_assign_pointer() because
other CPUs are unable to access the data structure -- only the final
assignment that publishes the whole group need be rcu_assign_pointer().
On the other hand, the cost of the extra memory barrier would be
insignificant in most cases.

Ah. Yeah, but we probably need a "raw" accessor anyway if we're going to
go this route, e.g. for any deref within the update-locked section.

I've been playing a bit, see below for my play rcupdate.h and test.c
test program.

Unfortunately, sparse doesn't have the ability to declare
"__attribute__((force_bitwise)) typeof(p)" or even
"__attribute__((force)) typeof(p)" which makes this force more than
necessary and causes it to not catch when incompatible pointers are
used. gcc notices that because I only do a cast at all for sparse, but
that doesn't help, since e.g. list_for_each_entry_rcu() requires that
the correct type is returned. So without sparse supporting the latter
notation, we don't stand a chance.

"<feff>"???

Hmm?

Also, I wouldn't know how to declare that an array or so needs
rcu-access to the members.

Hmmm... Can you apply the address-space attribute to the array itself?
I suppose one could convert the array to a pointer, but yecch!

Not sure if applying that to an array would work, and I wouldn't want to
convert it to pointers either. But I suppose you could declare the array
like this:

static struct foo * __attribute__((bitwise or address_space)) array[7];

which should, as far as I understand, apply the attribute to the array
members instead of the array.

johannes

Attachment: signature.asc
Description: This is a digitally signed message part

Relevant Pages

  • Re: memory leak help!
    ... I don't have any pointers in my program ... at all...you have to declare them as type POINTER, ... the function similarity is called from within a loop of the main ... array to be sorted ...
    (comp.lang.fortran)
  • Re: Enumerated types
    ... Enum ACGroup ... I cannot then access the members of the group. ... One alternative I could try, in the mean time, is to declare a string array ...
    (microsoft.public.access.formscoding)
  • Re: Differance between Array and Pointers
    ... Well, except that arrays tend to be much larger than pointers, and the ... An array is a contiguous region of memory containing N elements of M ... indexing vs. a loop). ...
    (comp.arch.embedded)
  • [RFCv2][PATCH] flexible array implementation
    ... I call it a flexible array. ... storage for pointers to the second level. ... all locking must be provided by the caller. ... make sure to pass in &ptr instead of ptr. ...
    (Linux-Kernel)
  • Re: [RFCv2][PATCH] flexible array implementation
    ... I call it a flexible array. ... storage for pointers to the second level. ... all locking must be provided by the caller. ... make sure to pass in &ptr instead of ptr. ...
    (Linux-Kernel)
Loading