On Tue, Aug 17, 2010 at 5:01 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
> Le mardi 17 août 2010 à 16:46 +0800, Changli Gao a écrit :
>> On Tue, Aug 17, 2010 at 4:30 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote:
>> >
>> > Three variables ?
>> >
>> > static atomic_t rnd __read_mostly;
>> >
>> > if (unlikely(!atomic_read(&rnd))) {
>> > unsigned int val;
>> >
>> > get_random_bytes(&val, sizeof(val));
>> > if (!val)
>> > val = 1;
>> > atomic_cmpxchg(&rnd, 0, val);
>> > }
>> >
>>
>>
>> Good idea. However, atomic_t is a volatile variable, and it prevent
>> ILP. I think maybe it hurts the likely case. cmpxchg() is an atomic
>> operations, so is the bellow code better?
>>
>
> I am not sure what you mean by ILP.
I mean http://en.wikipedia.org/wiki/Instruction_level_parallelism
and volatile will suppress compiler optimization.
>
> On x86, reading twice same memory location is faster than
> reading it and store in temp variable (on stack)
> reading from stack
Thanks for this info. But compiler may store it in a register. In
fact, we can't control the behavior of compiler in C.
>
>> static unsigned long rnd __read_mostly;
>>
>> if (unlikely(!rnd)) {
>> unsigned long val;
>>
>> get_random_bytes(&val, sizeof(val));
>> if (!val)
>> val = 1;
>> cmpxchg(&rnd, 0, val);
>> }
>>
>> Thanks.
>>
>
> I am not sure we must use a long (we really need 4 bytes only),
Yea, 4 bytes is enough.
> and last
> time I tried to use cmpxchg(), I was being told it was not available on
> all arches.
>
> But seeing it used in kernel/pid.c, maybe its not true anymore (that is,
> __HAVE_ARCH_CMPXCHG is always defined to 1)
>
> Since its a recent change (in kernel/pid.c), I would wait a bit and see
> if an arch maintainer complains ;)
>
Thanks. I saw it. These are two cmpxchg() in file:
include/asm-generic/cmpxchg.h
include/asm-generic/system.h
static inline unsigned long __cmpxchg(volatile unsigned long *m,
unsigned long old, unsigned long new)
{
unsigned long retval;
unsigned long flags;
local_irq_save(flags);
retval = *m;
if (retval == old)
*m = new;
local_irq_restore(flags);
return retval;
}
#define cmpxchg(ptr, o, n) \
((__typeof__(*(ptr))) __cmpxchg((unsigned long *)(ptr), \
(unsigned long)(o), \
(unsigned long)(n)))
This one seems buggy when using it to non unsigned long variables. And
in kernel/pid.c, the last_pid variable is int.
--
Regards,
Changli Gao(xiaosuo@xxxxxxxxx)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
