Re: [PATCH] percpu,lockdep: implement and use is_static_percpu_address()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, 2010-03-09 at 18:12 +0900, Tejun Heo wrote:
> lockdep has custom code to check whether a pointer belongs to static
> percpu area which is somewhat broken.  Implement proper
> is_static_percpu_address() in percpu and replace the custom code.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
> This wasn't the cause this time but I'll queue the fix in percpu tree
> anyway.
> 
> Thanks.
> 
>  include/linux/percpu.h |    7 +++++++
>  kernel/lockdep.c       |   15 ++-------------
>  mm/percpu.c            |   26 ++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index a93e5bf..1eca064 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -137,6 +137,7 @@ extern int __init pcpu_page_first_chunk(size_t reserved_size,
>  extern void __percpu *__alloc_reserved_percpu(size_t size, size_t align);
>  extern void __percpu *__alloc_percpu(size_t size, size_t align);
>  extern void free_percpu(void __percpu *__pdata);
> +extern bool is_static_percpu_address(unsigned long addr);
>  extern phys_addr_t per_cpu_ptr_to_phys(void *addr);
>  
>  #ifndef CONFIG_HAVE_SETUP_PER_CPU_AREA
> @@ -163,6 +164,12 @@ static inline void free_percpu(void __percpu *p)
>  	kfree(p);
>  }
>  
> +/* can't distinguish from other static vars, always false */
> +static inline bool is_static_percpu_address(unsigned long addr)
> +{
> +	return false;
> +}

At this point it might make sense to simply fail to compile if lockdep
is enabled as well.

I'm not sure if there's many SMP archs that don't have this and do have
lockdep, but simply failing this test isn't really an option.

>  static inline phys_addr_t per_cpu_ptr_to_phys(void *addr)
>  {
>  	return __pa(addr);
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c

> diff --git a/mm/percpu.c b/mm/percpu.c
> index 768419d..ae4d058 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1304,6 +1304,32 @@ void free_percpu(void __percpu *ptr)
>  EXPORT_SYMBOL_GPL(free_percpu);
>  
>  /**
> + * is_static_percpu_address - test whether address is from static percpu area
> + * @addr: address to test
> + *
> + * Test whether @addr belongs to static percpu area.  Module static
> + * percpu areas allocated via __alloc_reserved_percpu() is not
> + * considered.  Use is_module_address() for those.
> + *
> + * RETURNS:
> + * %true if @addr is from static percpu area, %false otherwise.
> + */

So is_module_address() will only return true for static per-cpu module
storage, right?