upcoming percpu changes

upcoming percpu changes

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi Linus,

The percpu tree currently in linux-next
(git://git.kernel.org/pub/scm/linux/kernel/git/tj/percpu.git#for-next)
contains a patch to remove the "per_cpu_" prefix from percpu variables
followed by  a set of patches to annotate all the percpu variables (and
accessing variables) with a __percpu tag to put them into a separate
sparse name space.  These latter patches are causing some merge conflicts
with a couple of trees (and will most likely cause more before the merge
window).  A solution to this is for you to accept the patch below into
your tree now and then the annotating patches can be sent to the
respective maintainers directly.

Of course, this assumes that you will accept the percpu changes during
the next merge window (and they are not completely without controversy)
and the proponents actually submit them :-).

What do you think?

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 5 Feb 2010 16:09:11 +1100
Subject: [PATCH] percpu: add __percpu for sparse

This is to make the annotation of percpu variables during the next merge
window less painfull.

Extracted from a patch by Rusty Russell.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 include/linux/compiler.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 5be3dab..188fcae 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -15,6 +15,7 @@
 # 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)))
 extern void __chk_user_ptr(const volatile void __user *);
 extern void __chk_io_ptr(const volatile void __iomem *);
 #else
@@ -32,6 +33,7 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 # define __acquire(x) (void)0
 # define __release(x) (void)0
 # define __cond_lock(x,c) (c)
+# define __percpu
 #endif
 
 #ifdef __KERNEL__
-- 
1.6.6.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: upcoming percpu changes

[Tejun Heo]

Hello,

On 02/05/2010 02:16 PM, Stephen Rothwell wrote:
> The percpu tree currently in linux-next
> (git://git.kernel.org/pub/scm/linux/kernel/git/tj/percpu.git#for-next)
> contains a patch to remove the "per_cpu_" prefix from percpu variables
> followed by  a set of patches to annotate all the percpu variables (and
> accessing variables) with a __percpu tag to put them into a separate
> sparse name space.  These latter patches are causing some merge conflicts
> with a couple of trees (and will most likely cause more before the merge
> window).  A solution to this is for you to accept the patch below into
> your tree now and then the annotating patches can be sent to the
> respective maintainers directly.
> 
> Of course, this assumes that you will accept the percpu changes during
> the next merge window (and they are not completely without controversy)
> and the proponents actually submit them :-).
> 
> What do you think?
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 5 Feb 2010 16:09:11 +1100
> Subject: [PATCH] percpu: add __percpu for sparse
> 
> This is to make the annotation of percpu variables during the next merge
> window less painfull.
> 
> Extracted from a patch by Rusty Russell.
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

I agree that this would be the cleanest way to integrate things.
Thanks for doing this.  If this gets in, I'll send individual patches
to respective maintainers.

Acked-by: Tejun Heo <tj@kernel.org>

For reference, the thread which contains the annoation patches is

  http://lkml.org/lkml/2010/1/25/107

Thanks.

-- 
tejun

Re: upcoming percpu changes

[Ingo Molnar]

* Tejun Heo <tj@kernel.org> wrote:

> Hello,
> 
> On 02/05/2010 02:16 PM, Stephen Rothwell wrote:
> > The percpu tree currently in linux-next
> > (git://git.kernel.org/pub/scm/linux/kernel/git/tj/percpu.git#for-next)
> > contains a patch to remove the "per_cpu_" prefix from percpu variables
> > followed by  a set of patches to annotate all the percpu variables (and
> > accessing variables) with a __percpu tag to put them into a separate
> > sparse name space.  These latter patches are causing some merge conflicts
> > with a couple of trees (and will most likely cause more before the merge
> > window).  A solution to this is for you to accept the patch below into
> > your tree now and then the annotating patches can be sent to the
> > respective maintainers directly.
> > 
> > Of course, this assumes that you will accept the percpu changes during
> > the next merge window (and they are not completely without controversy)
> > and the proponents actually submit them :-).
> > 
> > What do you think?
> > 
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Fri, 5 Feb 2010 16:09:11 +1100
> > Subject: [PATCH] percpu: add __percpu for sparse
> > 
> > This is to make the annotation of percpu variables during the next merge
> > window less painfull.
> > 
> > Extracted from a patch by Rusty Russell.
> > 
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> I agree that this would be the cleanest way to integrate things.
> Thanks for doing this.  If this gets in, I'll send individual patches
> to respective maintainers.
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> 
> For reference, the thread which contains the annoation patches is
> 
>   http://lkml.org/lkml/2010/1/25/107
> 
> Thanks.

Has the review feedback from Frederic and hpa been addressed? (and have they 
acked the solution?) That was the only controversial bit i remember.

Thanks,

	Ingo

Re: upcoming percpu changes

[Tejun Heo]

Hello, Ingo.

On 02/05/2010 04:09 PM, Ingo Molnar wrote:
> Has the review feedback from Frederic and hpa been addressed? (and have they 
> acked the solution?) That was the only controversial bit i remember.

Al Viro said that, until sparse is improved, the only solution would
be to work around in the rather ugly way as posted in the original
patches.  As Frederic and hpa preferred not having the workaround and
there were only three instances in the kernel tree, I just left them
alone so sparse warning will trigger spuriously on those three
locations.

Thanks.

-- 
tejun

Re: upcoming percpu changes

[Ingo Molnar]

* Tejun Heo <tj@kernel.org> wrote:

> Hello, Ingo.
> 
> On 02/05/2010 04:09 PM, Ingo Molnar wrote:
> > Has the review feedback from Frederic and hpa been addressed? (and have they 
> > acked the solution?) That was the only controversial bit i remember.
> 
> Al Viro said that, until sparse is improved, the only solution would be to 
> work around in the rather ugly way as posted in the original patches.  As 
> Frederic and hpa preferred not having the workaround and there were only 
> three instances in the kernel tree, I just left them alone so sparse 
> warning will trigger spuriously on those three locations.
> 
> Thanks.

Ok, good!

	Ingo

Re: upcoming percpu changes

[Tejun Heo]

Hello, Stephen.

On 02/05/2010 02:16 PM, Stephen Rothwell wrote:
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 5 Feb 2010 16:09:11 +1100
> Subject: [PATCH] percpu: add __percpu for sparse
> 
> This is to make the annotation of percpu variables during the next merge
> window less painfull.
> 
> Extracted from a patch by Rusty Russell.

I started doing this and it's a bit ridiculous.  If I split the
patches into separate trees with maintainers, I end up with a lot of
one or several liners and all that those patches do is adding __percpu
to a variable or field declaration which doesn't affect normal builds
at all.  The only conflicts I had against the current mainline is the
ones which got changed in the percpu tree by Christoph's patches.

Given the wide number of trees this will end up on and given the
triviality of each change, I think it would better to keep these in
the percpu tree.  It'll make things harder track without adding much
benefit.  If non-trivial confict ever happens, please feel free to
drop it from linux-next and let me know.

Thanks.

-- 
tejun

Re: upcoming percpu changes

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]

Hi Tajun,

On Fri, 12 Feb 2010 17:21:20 +0900 Tejun Heo <tj@kernel.org> wrote:
>
> On 02/05/2010 02:16 PM, Stephen Rothwell wrote:
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Fri, 5 Feb 2010 16:09:11 +1100
> > Subject: [PATCH] percpu: add __percpu for sparse
> > 
> > This is to make the annotation of percpu variables during the next merge
> > window less painfull.
> > 
> > Extracted from a patch by Rusty Russell.
> 
> I started doing this and it's a bit ridiculous.  If I split the
> patches into separate trees with maintainers, I end up with a lot of
> one or several liners and all that those patches do is adding __percpu
> to a variable or field declaration which doesn't affect normal builds
> at all.  The only conflicts I had against the current mainline is the
> ones which got changed in the percpu tree by Christoph's patches.
> 
> Given the wide number of trees this will end up on and given the
> triviality of each change, I think it would better to keep these in
> the percpu tree.  It'll make things harder track without adding much
> benefit.  If non-trivial confict ever happens, please feel free to
> drop it from linux-next and let me know.

You don't need to split them all out, just the problematic ones.

A good start would be commit d3f5fa4075414c7754126fbdc7c8fbd3906db7c8
("percpu: add __percpu sparse annotations to net").  You would need to
rebase it on top of net-next and note that "struct macvlan_dev" has been
moved to include/linux/if_macvlan.h.  Then just send it to DaveM.

You could also send commit e20b68160144bcacbde008b9cf7df4e1544b2507
("percpu: add __percpu sparse annotations to net drivers") along with it.

Commit 949c49c93a02b62278a63457d0d3192d66e55105 ("percpu: add __percpu
sparse annotations to x86") can just go to the x86 guys and commit
0b6dd45041220083a3692edae5253c35d19bcb2b ("percpu: add __percpu sparse
annotations to hw_breakpoint") to Frederic Weisbecker.

Commit eeb721be6bc03fe37755e69ab5c3ba2fe9897fd9 ("percpu: add __percpu
sparse annotations to trace") needs rebasing onto the tip perf/core
branch and then could be sent to Steve and co.

That takes care of most of it and the rest, as you say, we can sort out
if it becomes a problem.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: upcoming percpu changes

[Tejun Heo]

Hello,

On 02/12/2010 08:10 PM, Stephen Rothwell wrote:
> A good start would be commit d3f5fa4075414c7754126fbdc7c8fbd3906db7c8
> ("percpu: add __percpu sparse annotations to net").  You would need to
> rebase it on top of net-next and note that "struct macvlan_dev" has been
> moved to include/linux/if_macvlan.h.  Then just send it to DaveM.
> 
> You could also send commit e20b68160144bcacbde008b9cf7df4e1544b2507
> ("percpu: add __percpu sparse annotations to net drivers") along with it.
> 
> Commit 949c49c93a02b62278a63457d0d3192d66e55105 ("percpu: add __percpu
> sparse annotations to x86") can just go to the x86 guys and commit
> 0b6dd45041220083a3692edae5253c35d19bcb2b ("percpu: add __percpu sparse
> annotations to hw_breakpoint") to Frederic Weisbecker.
> 
> Commit eeb721be6bc03fe37755e69ab5c3ba2fe9897fd9 ("percpu: add __percpu
> sparse annotations to trace") needs rebasing onto the tip perf/core
> branch and then could be sent to Steve and co.
> 
> That takes care of most of it and the rest, as you say, we can sort out
> if it becomes a problem.

The changes being basically trivial, it doesn't seem to bring much
benefit but yeah sure I can send those chunks to the maintainers.

Thanks.

-- 
tejun