Re: [PATCH 11/17] fuse: add number of waiting requests attribute

[Miklos Szeredi]

> This doesn't get initialised anywhere.
> 
> Presumably you're relying on a memset somewhere.  That might work on all
> architectures, AFAIK.  But in theory it's wrong.  If, for example, the
> architecture implements atomic_t via a spinlock-plus-integer, and that
> spinlock's unlocked state is not all-bits-zero, we're dead.
> 
> So we should initialise it with
> 
> 	foo->num_waiting = ATOMIC_INIT(0);
> 

Is it correct to use a structure initializer this way?

> nb: it is not correct to initialise an atomic_t with
> 
> 	atomic_set(a, 0);
> 
> because in the above theoretical case case where the arch uses a spinlock
> in the atomic_t, that spinlock doesn't get initialised.  I bet we've got code
> in there which does this.

According to Documentation/atomic_ops.txt, this is the correct usage
of atomic_set():

|           The first operations to implement for atomic_t's are the
|   initializers and plain reads.
|   
|           #define ATOMIC_INIT(i)          { (i) }
|           #define atomic_set(v, i)        ((v)->counter = (i))
|   
|   The first macro is used in definitions, such as:
|   
|   static atomic_t my_counter = ATOMIC_INIT(1);
|   
|   The second interface can be used at runtime, as in:
|   
|           struct foo { atomic_t counter; };
|           ...
|   
|           struct foo *k;
|   
|           k = kmalloc(sizeof(*k), GFP_KERNEL);
|           if (!k)
|                   return -ENOMEM;
|           atomic_set(&k->counter, 0);

So in fact atomic_set() is an initializer, and should be named
atomic_init() accordingly.  Is atomic_set() ever used as an atomic
operation rather than an initializer?

Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/