On Fri, Nov 29, 2019 at 06:11:59PM +0000, Christopher Lameter wrote:
> On Wed, 27 Nov 2019, Luc Van Oostenryck wrote:
>
> > 1) it would strip any address space, not just __percpu, so:
> > it would need to be combined with __verify_pcpu_ptr() or,
> > * a better name should be used,
>
> typeof_cast_kernel() to express the fact that it creates a kernel pointer
> and ignored the attributes??
typeof_strip_address_space() would, I think, express this better.
It's not obvious at all to me that 'kernel' in 'typeof_cast_kernel()'
relates to the (default) kernel address space.
Maybe it's just me. I don't know.
> > * it should be defined in a generic header, any idea where?
>
> include/linux/compiler-types.h
Yes, OK.
> > 2) while I find the current solution:
> > typeof(T) __kernel __force *ptr = ...;
>
> It would be
>
> typeof_cast_kernel(&T) *xx = xxx
>
> or so?
No, it would not. __percpu, and more generally, the address space
is a property of the object, not of its address.
For example, let's say T is a __percpu object:
int __percpu obj;
then '&T' is just a 'normal'/__kernel pointer to it:
int __percpu *;
There is nothing to strip (it would be if the __percpu
would be 'on the other side of the *': int * __percpu).
It's exactly the same as with 'const': a 'const char *'
is not const, only a pointer to const.
The situation with raw_cpu_generic_add_return() is:
- pcp is a lvalue of of a __percpu object of type T, so:
typeof(pcp) := T __percpu
- pcp's address is given to raw_cpu_ptr(), so
typeof(&pcp) := T __percpu *
- raw_cpu_ptr() return the corresponding __kernel pointer
(adjusted for the current percu offset), so:
typeof(raw_cpu_ptr(&pcp)) := T *
- so, the macro needs to declare a variable __p of type T*
hence:
typeof(pcp) __kernel __force *__p;
or, with this new macro:
typeof_cast_kernel(pcp) *__p;
Maybe a better solution would be to directly play at pointer
level and thus have something like this:
typeof_<some good name>(&pcp) __p = raw_cpu_ptr(&pcp);
or even:
__kernel_pointer(&pcp) __p = raw_cpu_ptr(&pcp);
I dunno.
Note: at implementation level, it complicates things slightly
to want this 'strip_percpu' macro to behaves like typeof()
because it means that it can take in argument either an
expression or a type. And if it's a type, you can't do a
simple cast on it, you need to declare an intermediate
variable, hence the horrible:
typeof(({ typeof(T) __kernel __force __fakename; __fakename; }))
Note: it would be much much nicer to do all these type generic
macros with '__auto_type' (only supported in GCC 4.9 IIUC
and supported in sparse but it shouldn't be very hard to do)..
-- Luc
