> 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.
> I recently tried to modify rcu_assign_pointer() to issue the memory
> memory barrier only when the pointer was non-NULL, but this ended badly.
Hm? I thought that's in the current tree.
> Probably because I am not the greatest gcc expert around... We ended
> up having to define an rcu_assign_index() to handle the possibility of
> assigning a zero-value array index, but my attempts to do type-checking
> backfired, and I eventually gave it up. Again, someone a bit more clued
> in to gcc than I am could probably pull it off.
Ah, ok.
> In addition, it is legal to omit rcu_dereference() and rcu_assign_pointer()
> when holding the update-side lock.
That I don't understand. Well, I do understand that omitting
rcu_dereference() is ok, but it seems to me that the memory and compiler
barrier in rcu_assign_pointer() is actually needed.
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.
Also, I wouldn't know how to declare that an array or so needs
rcu-access to the members.
johannes
rcupdate.h:
#define USE_BITWISE
#ifdef __CHECKER__
#ifdef USE_BITWISE
#define __rcu __attribute__((bitwise))
#define __force_rcu_cast(p) (*((__attribute__((force)) void **)&(p)))
// would like instead:
//#define __force_rcu_cast(p) ((__attribute__((force_bitwise)) typeof(p)) (p))
#else /* not bitwise */
#define __rcu __attribute__((address_space(3)))
#define __force_rcu_cast(p) (*((__attribute__((force)) void **)&(p)))
// would like instead:
//#define __force_rcu_cast(p) ((__attribute__((force_address_space)) typeof(p)) (p))
#endif
#else /* not checker */
#define __rcu
#define __force_rcu_cast(p) (p)
#endif
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
#define rcu_dereference(p) ({ \
typeof(p) _________p1 = ACCESS_ONCE(p); \
smp_read_barrier_depends(); \
__force_rcu_cast(_________p1); \
})
/**
* rcu_fetch - fetch an RCU-protected pointer in the update-locked
* critical section.
*
* This macro exists for documentation and code checking purposes.
*/
#define rcu_fetch(p) __force_rcu_cast(p);
#define rcu_assign_pointer(p, v) \
({ \
if (!__builtin_constant_p(v) || \
((v) != NULL)) \
smp_wmb(); \
__force_rcu_cast(p) = (v); \
})
test.c:
#include <stdlib.h>
#include "rcupdate.h"
/* my rcu protected variables */
static unsigned int __rcu *prot;
static unsigned int __rcu *prot_same;
static unsigned char __rcu *prot2;
// dummies
static smp_read_barrier_depends(void) {}
static smp_wmb(void) {}
int main(void)
{
unsigned int *tmp;
// no warnings from sparse due to forced cast
rcu_assign_pointer(prot, tmp);
// but gcc warns
rcu_assign_pointer(prot2, tmp);
// no warnings
rcu_assign_pointer(prot, NULL);
rcu_assign_pointer(prot2, NULL);
// no warnings
prot = NULL;
prot2 = NULL;
// no warnings from sparse due to forced cast
tmp = rcu_dereference(prot);
// but gcc warns
tmp = rcu_dereference(prot2);
/* now within locked section rcu_dereference isn't required */
// no warnings from sparse due to forced cast
tmp = rcu_fetch(prot);
// but gcc warns
tmp = rcu_fetch(prot2);
/* not caught with address_space, but is caught with bitwise */
prot = prot_same;
}
Attachment:
signature.asc
Description: This is a digitally signed message part
