Subject: Re: [PATCH 7/8] percpu: add __percpu sparse annotations to hw_breakpoint

From: Frederic Weisbecker <[email-masked]>
Date: Tue, 26 Jan 2010 03:01:14 +0100

On Tue, Jan 26, 2010 at 10:19:04AM +0900, Tejun Heo wrote:
> Hello,
> 
> On 01/26/2010 10:02 AM, Frederic Weisbecker wrote:
> > Well, sorry I must be missing something obvious, but is it impossible
> > to make per_cpu(var, cpu) returning something cast in:
> > 
> > 	(typeof(var) __force)
> > 
> > Or I guess you did that already and it is not working with static
> > arrays, or?
> 
> Yeap, the definition looks like
> 
>  #define SHIFT_PERCPU_PTR(__p, __offset)	({			\
> 	__verify_pcpu_ptr((__p));					\
> 	RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)); \
>  })
> 
>  #define per_cpu(var, cpu) \
> 	(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))
> 
> but it just ends up putting the __force at the wrong layer.  It seems
> that (typeof(var) __kernel __force) tell sparse var is in the kernel
> address space but not its members.

So, may be it considers you are applying the address space overriding
to the pointer to the type and not to the type itself.

Consider:

	int __percpu i;

What you do above *might* be considered as if SHIFT_PERCPU_PTR
returns something of a type:

	int * __percpu i;

So the pointer is in the normal address space, but its content is in
__percpu address space.

What if you do this:

#define SHIFT_PERCPU_PTR(__p, __offset)      ({                      \
      __verify_pcpu_ptr((__p));                                       \
      RELOC_HIDE((__p), (__offset)); \
})

#define per_cpu(var, cpu) \
      (typeof(var) __kernel __force)(*SHIFT_PERCPU_PTR(&(var), per_cpu_offset(cpu)))

This should work because &(var) should be dereferencable directly, since
it is not of type "__force t" but of type "*__force t"

And you're not doing anymore this:

	*(int * __kernel __force) i;
but
	*(int __kernel __force *) i;

That all might make no sense, I'm just trying to think like a backend
so it might sound like I should just take more sleep and just shut up...