Messages in this thread | | | Date | Thu, 15 Oct 2009 18:24:39 +0900 | From | Tejun Heo <> | Subject | Re: [PATCH 13/16] percpu: remove per_cpu__ prefix. |
| |
(microblaze maintainer cc'd, hello)
Christoph Lameter wrote: > On Wed, 14 Oct 2009, Tejun Heo wrote: > >> @@ -39,7 +39,7 @@ extern void *per_cpu_init(void); >> * On the positive side, using __ia64_per_cpu_var() instead of __get_cpu_var() is slightly >> * more efficient. >> */ >> -#define __ia64_per_cpu_var(var) per_cpu__##var >> +#define __ia64_per_cpu_var(var) var > > IA64 could completely drop the macro? Tony?
Being discussed but I think we should just add sparse annotation there instead.
>> diff --git a/arch/microblaze/include/asm/entry.h b/arch/microblaze/include/asm/entry.h >> index 61abbd2..ec89f2a 100644 >> --- a/arch/microblaze/include/asm/entry.h >> +++ b/arch/microblaze/include/asm/entry.h >> @@ -21,7 +21,7 @@ >> * places >> */ >> >> -#define PER_CPU(var) per_cpu__##var >> +#define PER_CPU(var) var > > Microblaze too.
This macro is used only in assemblies which wouldn't be covered by sparse so in this case this patch series actually removes protection, so I wasn't too sure about ripping the macro off. Any ideas what we can do here? Just kill it?
>> +#define PER_CPU(var, reg) __percpu_mov_op $var, reg >> +#define PER_CPU_VAR(var) var > > Drop X86 PER_CPU_VAR
No can do. SMP variant isn't null op.
>> -#define percpu_read(var) percpu_from_op("mov", per_cpu__##var, \ >> - "m" (per_cpu__##var)) >> -#define percpu_read_stable(var) percpu_from_op("mov", per_cpu__##var, \ >> - "p" (&per_cpu__##var)) >> -#define percpu_write(var, val) percpu_to_op("mov", per_cpu__##var, val) >> -#define percpu_add(var, val) percpu_to_op("add", per_cpu__##var, val) >> -#define percpu_sub(var, val) percpu_to_op("sub", per_cpu__##var, val) >> -#define percpu_and(var, val) percpu_to_op("and", per_cpu__##var, val) >> -#define percpu_or(var, val) percpu_to_op("or", per_cpu__##var, val) >> -#define percpu_xor(var, val) percpu_to_op("xor", per_cpu__##var, val) >> +#define percpu_read(var) percpu_from_op("mov", var, "m" (var)) >> +#define percpu_read_stable(var) percpu_from_op("mov", var, "p" (&(var))) >> +#define percpu_write(var, val) percpu_to_op("mov", var, val) >> +#define percpu_add(var, val) percpu_to_op("add", var, val) >> +#define percpu_sub(var, val) percpu_to_op("sub", var, val) >> +#define percpu_and(var, val) percpu_to_op("and", var, val) >> +#define percpu_or(var, val) percpu_to_op("or", var, val) >> +#define percpu_xor(var, val) percpu_to_op("xor", var, val) > > The percpu_xx definitions are now equal to __this_cpu_xx(). They could be > dropped for the core.
Yeap, will do so with further patches.
>> #define __get_cpu_var(var) \ >> - (*SHIFT_PERCPU_PTR(&per_cpu_var(var), my_cpu_offset)) >> + (*SHIFT_PERCPU_PTR(&(var), my_cpu_offset)) > > == this_cpu_read(var) or this_cpu_write(var, value) > >> #define __raw_get_cpu_var(var) \ >> - (*SHIFT_PERCPU_PTR(&per_cpu_var(var), __my_cpu_offset)) >> + (*SHIFT_PERCPU_PTR(&(var), __my_cpu_offset)) > > == __this_cpu_read() or this_cpu_write(var, value) > > __raw? Combination of __ and raw? Can we clearly define what each means? > >> - typeof(per_cpu_var(var)) __tmp_var__; \ >> + typeof(var) __tmp_var__; \ >> __tmp_var__ = get_cpu_var(var); \ >> put_cpu_var(var); \ >> __tmp_var__; \ > > == this_cpu_read(var)
For all of above comments, yeap, we definitely need to clean all these up, but let's do that once sparse annotation is working.
> Great work. There is lots more possible cleanup work that could be done > after this patch has merged. > > Reviewed-by: Christoph Lameter <cl@linux-foundation.org>
Thanks.
-- tejun
| |