To: akpm@osdl.org
CC: linux-kernel@vger.kernel.org
In-reply-to: <20060113172846.3ea49670.akpm@osdl.org> (message from Andrew
	Morton on Fri, 13 Jan 2006 17:28:46 -0800)
Subject: Re: [PATCH 11/17] fuse: add number of waiting requests attribute
References: <20060114003948.793910000@dorka.pomaz.szeredi.hu>
	<20060114004114.241169000@dorka.pomaz.szeredi.hu> <20060113172846.3ea49670.akpm@osdl.org>
Message-Id: <E1ExhxA-0004Bz-00@dorka.pomaz.szeredi.hu>
From: Miklos Szeredi <miklos@szeredi.hu>
Date: Sat, 14 Jan 2006 10:44:08 +0100
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1887
Lines: 57

> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/