Re: [PATCH 2/5] atomic: introduce atomic_inc_not_zero

[Roman Zippel]

Hi,

On Wed, 14 Sep 2005, Russell King wrote:

> > 
> > > > 	do {
> > > > 		old = atomic_load_locked(v);
> > > > 		if (!old)
> > > > 			break;
> > > > 		new = old + 1;
> > > > 	} while (!atomic_store_lock(v, old, new));
> > > 
> 
> What you're asking architectures to do is:
> 
> retry:
> 	load
> 	operation
> 	save interrupts
> 	load
> 	compare
> 	store if equal
> 	restore interrupts
> 	goto retry if not equal
> 
> whereas they could have done the far simpler version of:
> 
> 	save interrupts
> 	load
> 	operation
> 	store
> 	restore interrupts
> 
> which they do today.

So modify it this way:

	atomic_lock(flags);
	do {
		old = atomic_load_locked(v);
		if (!old)
			break;
		new = old + 1;
	} while (!atomic_store_locked(v, old, new));
	atomic_unlock(flags);

> The whole point about architecture specific includes is not to provide
> a frenzied feeding ground for folk who like to "clean code up" but to
> allow architectures to do things in the most efficient way for them
> without polluting the kernel too much.

In this case it's massively repeated code which only differs slighty. My 
biggest problem here is the lack of gcc support to get the condition code 
out of an asm. Especially architectures with locked load/store instruction 
could then have just a single implementation in asm-generic.

A long time ago I was playing with something like this:

#define atomic_exchange(ptr, old, new) ({		\
	old = *(ptr);					\
	asm volatile ("1:"				\
		: "=&a" (old), "=m" (*(ptr))		\
		: "0" (old), "m" (*(ptr))		\
		: "memory");				\
	asm volatile (__LOCK "cmpxchg %2,%0; jne 1b"	\
		: "=m" (*(ptr)), "=&a" (old)		\
		: "r" (new), "m" (*(ptr)), "1" (old)	\
		: "memory");				\
})

so e.g. an atomic_add_return() would be atomic_exchange(ptr, old, old + 
val), but I guess gcc people would kill me for that. :-)

bye, Roman
-
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/