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

[Eric Dumazet <dada1@cosmosbay.com>]

Andrew Morton a écrit :
> Miklos Szeredi <miklos@szeredi.hu> wrote:
> > +	/** The number of requests waiting for completion */
> > +	atomic_t num_waiting;
>
> 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);
>
>
>
> 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.

Hum... I tracked one missing atomic_set() or ATOMIC_INIT in e1000 driver then.

e1000_alloc_queues() does :

#ifdef CONFIG_E1000_NAPI
        size = sizeof(struct net_device) * adapter->num_queues;
        adapter->polling_netdev = kmalloc(size, GFP_KERNEL);
        if (!adapter->polling_netdev) {
                kfree(adapter->tx_ring);
                kfree(adapter->rx_ring);
                return -ENOMEM;
        }
        memset(adapter->polling_netdev, 0, size);
#endif

So this driver clearly assumes a memset(... 0 ...) also initialize atomic_t to 
 0   ((struct net_device *)->refcnt for example)

Eric
-
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/