Prev: still worth running kernel cleanup scripts after merge windows?
Next: fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues
From: Peter Zijlstra on 9 Mar 2010 06:30
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(a)kernel.org>
> Cc: Ingo Molnar <mingo(a)redhat.com>
> Cc: Peter Zijlstra <peterz(a)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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 9 Mar 2010 07:00
On Tue, 2010-03-09 at 20:42 +0900, Tejun Heo wrote:
> Hello, Peter.
>
> On 03/09/2010 08:25 PM, Peter Zijlstra wrote:
> >> +/* 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.
>
> That might be better. Returning %false isn't that bad tho. There
> really is no distinction between percpu and !percpu variable on UP and
> static variable address match will catch both.

Ah, if this is UP only then yes, no complaints ;-)

> >> /**
> >> + * 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?
>
> Right, got confused there. I'll update is_static_percpu_address() to
> test for reserved regions too.

Or as per your other email, make the above be true, and have
is_module_address() be true for its static per-cpu regions.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: still worth running kernel cleanup scripts after merge windows?
Next: fix sync_mm_rss in nommu (Was Re: sync_mm_rss() issues