Messages in this thread Patch in this message | | | From | Arnd Bergmann <> | Subject | Re: [RFC local_t removal V1 1/4] Add add_local() and add_local_return() | Date | Thu, 7 Jan 2010 14:45:50 +0100 |
| |
On Tuesday 05 January 2010, Mathieu Desnoyers wrote: > > The problem I see here is that with ~5-6 operations, we will end up > having 20*5 = 100 headers only for this. Can we combine these in a > single header file instead ? local.h wasn't bad in this respect.
I have an old patch that I was planning to dig out for 2.6.34, which autogenerates arch/*/include/foo.h files that only contain "#include <asm-generic/foo.h>".
I guess this would be sufficient to avoid the overload with all these header files.
> Also, separating all these in sub-files will make it a bit difficult to > pinpoint errors that would arise from using a bad combination of, e.g. > generic add with a non-protected read or set.
Yes.
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > > +++ linux-2.6/include/asm-generic/add-local-generic.h 2010-01-05 15:36:02.000000000 -0600 > > @@ -0,0 +1,40 @@ > > +#ifndef __ASM_GENERIC_ADD_LOCAL_GENERIC_H > > +#define __ASM_GENERIC_ADD_LOCAL_GENERIC_H Why split the file between asm-generic/add-local.h and add-local-generic.h? I don't see how any architecture could use one but not the other.
> > +#include <linux/types.h> > > + > > +extern unsigned long wrong_size_add_local(volatile void *ptr); > > + > > +/* > > + * Generic version of __add_return_local (disables interrupts). Takes an > > + * unsigned long parameter, supporting various types of architectures. > > + */ > > +static inline unsigned long __add_return_local_generic(volatile void *ptr, > > + unsigned long value, int size)
You could probably lose the 'volatile' here, if you want to discourage marking data as volatile in the code.
> > +{ > > + unsigned long flags, r; > > + > > + /* > > + * Sanity checking, compile-time. > > + */ > > + if (size == 8 && sizeof(unsigned long) != 8) > > + wrong_size_add_local(ptr);
It can be BUILD_BUG_ON if you move it to the outer macro.
> > + local_irq_save(flags); > > + switch (size) { > > + case 1: r = (*((u8 *)ptr) += value); > > + break; > > + case 2: r = (*((u16 *)ptr) += value); > > + break; > > + case 4: r = (*((u32 *)ptr) += value); > > + break; > > + case 8: r = (*((u64 *)ptr) += value); > > + break;
But I think here you actually need to add the volatile in order to make these atomic assignments.
Arnd
| |