Messages in this thread | | | Date | Thu, 15 Oct 2009 18:08:28 +0900 | From | Tejun Heo <> | Subject | Re: [PATCH 16/16] percpu: make accessors check for percpu pointer in sparse |
| |
Christoph Lameter wrote: > On Wed, 14 Oct 2009, Tejun Heo wrote: > >> #ifndef SHIFT_PERCPU_PTR >> /* Weird cast keeps both GCC and sparse happy. */ >> -#define SHIFT_PERCPU_PTR(__p, __offset) \ >> - RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)) >> +#define SHIFT_PERCPU_PTR(__p, __offset) ({ \ >> + __verify_pcpu_ptr((__p)); \ >> + RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \ >> +}) > > If you have the verification in SHIFT_PER_CPU_PTR then why do you need it > elsewhere?
Because this_cpu_*() macros might not calculate addresses using SHIFT_PERCPU_PTR().
>> #define __pcpu_size_call_return(stem, variable) \ >> ({ typeof(variable) pscr_ret__; \ >> + __verify_pcpu_ptr(&(variable)); \ >> switch(sizeof(variable)) { \ >> case 1: pscr_ret__ = stem##1(variable);break; \ >> case 2: pscr_ret__ = stem##2(variable);break; \ >> @@ -250,6 +251,7 @@ extern void __bad_size_call_parameter(void); >> >> #define __pcpu_size_call(stem, variable, ...) \ >> do { \ >> + __verify_pcpu_ptr(&(variable)); \ >> switch(sizeof(variable)) { \ >> case 1: stem##1(variable, __VA_ARGS__);break; \ >> case 2: stem##2(variable, __VA_ARGS__);break; \ > > Would it not be better to put the verification in the arch code? The > percpu_to/from_op may have multiple callsites (at least they have now). If > you put it in there then all other stuff is covered.
I don't know. The way these ops are defined, adding __verify_pcpu_ptr() to size_call macros reliably cover all percpu cases and I much prefer things like this being done in generic code rather than requiring each arch to do it. It's just more reliable this way.
Thanks.
-- tejun
| |