2010-08-06 12:46:17

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH] percpu: remove address_space attribute if !SMP

percpu data has no special meaning in case of !CONFIG_SMP.
This removes lots of sparse warnings.

Signed-off-by: Namhyung Kim <[email protected]>

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a5a472b..8dfd41d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -15,7 +15,11 @@
# define __acquire(x) __context__(x,1)
# define __release(x) __context__(x,-1)
# define __cond_lock(x,c) ((c) ? ({ __acquire(x); 1; }) : 0)
-# define __percpu __attribute__((noderef, address_space(3)))
+# ifdef CONFIG_SMP
+# define __percpu __attribute__((noderef, address_space(3)))
+# else
+# define __percpu
+# endif
extern void __chk_user_ptr(const volatile void __user *);
extern void __chk_io_ptr(const volatile void __iomem *);
#else
--
1.7.0.4


2010-08-06 12:49:54

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

On 08/06/2010 02:46 PM, Namhyung Kim wrote:
> percpu data has no special meaning in case of !CONFIG_SMP.
> This removes lots of sparse warnings.
>
> Signed-off-by: Namhyung Kim <[email protected]>

But they should still be accessed through the accessors and if they
are accessed through accessors, there shouldn't be sparse warnings
regarding them. Maybe UP accessors are missing proper markups? Do
those warnings only happen on UP config?

--
tejun

2010-08-06 12:53:26

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

2010-08-06 (금), 14:49 +0200, Tejun Heo:
> On 08/06/2010 02:46 PM, Namhyung Kim wrote:
> > percpu data has no special meaning in case of !CONFIG_SMP.
> > This removes lots of sparse warnings.
> >
> > Signed-off-by: Namhyung Kim <[email protected]>
>
> But they should still be accessed through the accessors and if they
> are accessed through accessors, there shouldn't be sparse warnings
> regarding them. Maybe UP accessors are missing proper markups? Do
> those warnings only happen on UP config?
>

They do nothing on UP.
quoting from include/asm-generic.h:

#else /* ! SMP */

#define per_cpu(var, cpu) (*((void)(cpu), &(var)))
#define __get_cpu_var(var) (var)
#define __raw_get_cpu_var(var) (var)
#define this_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
#define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)

#endif /* SMP */


--
Regards,
Namhyung Kim

2010-08-06 12:57:22

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

Hello,

On 08/06/2010 02:53 PM, Namhyung Kim wrote:
>> But they should still be accessed through the accessors and if they
>> are accessed through accessors, there shouldn't be sparse warnings
>> regarding them. Maybe UP accessors are missing proper markups? Do
>> those warnings only happen on UP config?
>>
>
> They do nothing on UP.
> quoting from include/asm-generic.h:
>
> #else /* ! SMP */
>
> #define per_cpu(var, cpu) (*((void)(cpu), &(var)))
> #define __get_cpu_var(var) (var)
> #define __raw_get_cpu_var(var) (var)
> #define this_cpu_ptr(ptr) per_cpu_ptr(ptr, 0)
> #define __this_cpu_ptr(ptr) this_cpu_ptr(ptr)
>
> #endif /* SMP */

Ah, I see. Then, the right thing to do is to add proper checking and
markups to UP accessors matching the SMP ones.
ie. __verify_pcpu_ptr() verification followed by __kernel __force
casting. Are you interested in doing it?

Thanks.

--
tejun

2010-08-06 13:23:41

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

2010-08-06 (금), 14:57 +0200, Tejun Heo:

> Ah, I see. Then, the right thing to do is to add proper checking and
> markups to UP accessors matching the SMP ones.
> ie. __verify_pcpu_ptr() verification followed by __kernel __force
> casting. Are you interested in doing it?
>
> Thanks.
>

Sure. :-)
Although I'm not sure that it is really needed, if so I'll do that.

--
Regards,
Namhyung Kim

2010-08-06 13:37:35

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

Hello,

On 08/06/2010 03:23 PM, Namhyung Kim wrote:
> Sure. :-)
> Although I'm not sure that it is really needed, if so I'll do that.

Well, the whole __percpu markup thing is to detect misuse of percpu
pointers. I think it would definitely be better to have the sanity
check for UP code too. It's not like it's gonna cost any runtime
overhead and all the needed pieces are already there.

Thanks.

--
tejun

2010-08-06 13:46:18

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] percpu: remove address_space attribute if !SMP

2010-08-06 (금), 15:37 +0200, Tejun Heo:
> Hello,
>

Hi,

> On 08/06/2010 03:23 PM, Namhyung Kim wrote:
> > Sure. :-)
> > Although I'm not sure that it is really needed, if so I'll do that.
>
> Well, the whole __percpu markup thing is to detect misuse of percpu
> pointers. I think it would definitely be better to have the sanity
> check for UP code too. It's not like it's gonna cost any runtime
> overhead and all the needed pieces are already there.
>
> Thanks.
>

I see. Thanks for the comments.

--
Regards,
Namhyung Kim