linux-next: sparc tree build failure

linux-next: sparc tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (sparc64 defconfig) failed like this:

arch/sparc/kernel/built-in.o: In function `trap_init':
(.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'

Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
requeue_pi functionality") (from the tip-core tree) which changed the
size of struct restart_block.

Dave, this might be a good time to suggest that sparc64 create its TI_
offsets automatically via asm-offsets ...

I applied the following patch for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

>From 11f2623cf0416509193865acad7e36ae05e31b1a Mon Sep 17 00:00:00 2001
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 9 Apr 2009 15:12:38 +1000
Subject: [PATCH] sparc64: update for restart_block size change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/sparc/include/asm/thread_info_64.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/thread_info_64.h b/arch/sparc/include/asm/thread_info_64.h
index 639ac80..6586572 100644
--- a/arch/sparc/include/asm/thread_info_64.h
+++ b/arch/sparc/include/asm/thread_info_64.h
@@ -102,8 +102,8 @@ struct thread_info {
 #define TI_KERN_CNTD1	0x00000488
 #define TI_PCR		0x00000490
 #define TI_RESTART_BLOCK 0x00000498
-#define TI_KUNA_REGS	0x000004c0
-#define TI_KUNA_INSN	0x000004c8
+#define TI_KUNA_REGS	0x000004c8
+#define TI_KUNA_INSN	0x000004d0
 #define TI_FPREGS	0x00000500
 
 /* We embed this in the uppermost byte of thread_info->flags */
-- 
1.6.2.1

Re: linux-next: sparc tree build failure

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
> arch/sparc/kernel/built-in.o: In function `trap_init':
> (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'
> 
> Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
> requeue_pi functionality") (from the tip-core tree) which changed the
> size of struct restart_block.
> 
> Dave, this might be a good time to suggest that sparc64 create its TI_
> offsets automatically via asm-offsets ...
> 
> I applied the following patch for today.

David, would it be appropriate for us to queue up Stephen's fix in 
the futex tree, so that any rewrite can be decoupled from these 
changes and the Sparc tree would still work as well?

	Ingo

Re: linux-next: sparc tree build failure

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 9 Apr 2009 15:17:22 +1000

> Dave, this might be a good time to suggest that sparc64 create its TI_
> offsets automatically via asm-offsets ...

I want to know the exact offsets for cache layout reasons,
at the very least.  And there are other more important reasons
to check this stuff out by hand and don't let it compile
automatically.

Anything that changes the layout and size of members in struct
thread_info should be scrutinized carefully by every arch maintainer.

There are other implications for these kinds of changes, for example
this decreases the amount of space we have to save our recursive
kernel/user FPU state on sparc64.

I don't want any of this to be automated at all.

Re: linux-next: sparc tree build failure

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 9 Apr 2009 07:27:17 +0200

> 
> * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
>> Hi all,
>> 
>> Today's linux-next build (sparc64 defconfig) failed like this:
>> 
>> arch/sparc/kernel/built-in.o: In function `trap_init':
>> (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'
>> 
>> Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
>> requeue_pi functionality") (from the tip-core tree) which changed the
>> size of struct restart_block.
>> 
>> Dave, this might be a good time to suggest that sparc64 create its TI_
>> offsets automatically via asm-offsets ...
>> 
>> I applied the following patch for today.
> 
> David, would it be appropriate for us to queue up Stephen's fix in 
> the futex tree, so that any rewrite can be decoupled from these 
> changes and the Sparc tree would still work as well?

Yes, but it's going to conflict with the dynamic per-cpu work
I'm doing in the sparc-next tree right now.

Re: linux-next: sparc tree build failure

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu, 9 Apr 2009 07:27:17 +0200
> 
> > 
> > * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > 
> >> Hi all,
> >> 
> >> Today's linux-next build (sparc64 defconfig) failed like this:
> >> 
> >> arch/sparc/kernel/built-in.o: In function `trap_init':
> >> (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'
> >> 
> >> Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
> >> requeue_pi functionality") (from the tip-core tree) which changed the
> >> size of struct restart_block.
> >> 
> >> Dave, this might be a good time to suggest that sparc64 create its TI_
> >> offsets automatically via asm-offsets ...
> >> 
> >> I applied the following patch for today.
> > 
> > David, would it be appropriate for us to queue up Stephen's fix 
> > in the futex tree, so that any rewrite can be decoupled from 
> > these changes and the Sparc tree would still work as well?
> 
> Yes, but it's going to conflict with the dynamic per-cpu work I'm 
> doing in the sparc-next tree right now.

Would it be possible for you to reserve the extra futex related 
pointer right now? (or is that not possible without changing the 
restart block in linux/thread_info.h?)

Last i checked the futex changes looked pretty ready: queued up for 
.31, they were ABI-acked by Ulrich, Roland, etc. and were tested 
through - so there's no additional churn or reverts / disappearing 
commits expected. (Thomas please confirm that this is indeed so.)

That way then when Stephen resolves the conflict then the futex tree 
change is overriden with your changes in the Sparc tree.

That will make it all bisectable as well - should anyone ever have 
the need to bisect down into that set of commits.

	Ingo

Re: linux-next: sparc tree build failure

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 9 Apr 2009 07:36:30 +0200

> Would it be possible for you to reserve the extra futex related 
> pointer right now? (or is that not possible without changing the 
> restart block in linux/thread_info.h?)

What are my real options?

No matter what, once you change the size of "struct restart_block"
the sparc64 build will fail, no matter what.

The only way around this is if you merge the thing that changes
the "struct restart_block"'s size to Linus now along with the
sparc64 change at the same time.

But then I have a merge issue I have to resolve in the sparc64
tree because I've already pushed out my dynamic per-cpu work
:-/

Grumble...

Re: linux-next: sparc tree build failure

[Ingo Molnar]

* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu, 9 Apr 2009 07:36:30 +0200
> 
> > Would it be possible for you to reserve the extra futex related 
> > pointer right now? (or is that not possible without changing the 
> > restart block in linux/thread_info.h?)
> 
> What are my real options?
> 
> No matter what, once you change the size of "struct restart_block" 
> the sparc64 build will fail, no matter what.
> 
> The only way around this is if you merge the thing that changes 
> the "struct restart_block"'s size to Linus now along with the 
> sparc64 change at the same time.
> 
> But then I have a merge issue I have to resolve in the sparc64 
> tree because I've already pushed out my dynamic per-cpu work
> :-/

We could create a standalone, .30-rc1 based topic for the 
linux/thread_info.h change only, and push it to Linus, and/or you 
could pull it into the Sparc tree?

	Ingo

Re: linux-next: sparc tree build failure

[David Miller]

From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 9 Apr 2009 07:53:56 +0200

> We could create a standalone, .30-rc1 based topic for the 
> linux/thread_info.h change only, and push it to Linus, and/or you 
> could pull it into the Sparc tree?

Actually, it turns out I'm changing different structure offsets in the
sparc64 dynamic per-cpu changes.  There shouldn't be any thread_info.h
conflicts, therefore.

So you can take the sparc64 build fix patch in your tree, that's fine.

Thanks!

[tip:core/futexes] sparc64: extend TI_RESTART_BLOCK space by 8 bytes

[Stephen Rothwell]

Commit-ID:  d2de688891909b148efe83a6fc9520a9cd6015f0
Gitweb:     http://git.kernel.org/tip/d2de688891909b148efe83a6fc9520a9cd6015f0
Author:     Stephen Rothwell <sfr@canb.auug.org.au>
AuthorDate: Thu, 9 Apr 2009 15:17:22 +1000
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 9 Apr 2009 08:09:50 +0200

sparc64: extend TI_RESTART_BLOCK space by 8 bytes

Impact: build fix

Today's linux-next build (sparc64 defconfig) failed like this:

  arch/sparc/kernel/built-in.o: In function `trap_init':
  (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'

Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
requeue_pi functionality") (from the tip-core tree) which changed the
size of struct restart_block.

Shift TI_KUNA_REGS and TI_KUNA_INSN up by 8 bytes to make space for the
larger restart block.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: "David S. Miller" <davem@davemloft.net>
Cc: Darren Hart <dvhltc@us.ibm.com>
LKML-Reference: <20090409151722.c8eabb56.sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/sparc/include/asm/thread_info_64.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/thread_info_64.h b/arch/sparc/include/asm/thread_info_64.h
index 639ac80..6586572 100644
--- a/arch/sparc/include/asm/thread_info_64.h
+++ b/arch/sparc/include/asm/thread_info_64.h
@@ -102,8 +102,8 @@ struct thread_info {
 #define TI_KERN_CNTD1	0x00000488
 #define TI_PCR		0x00000490
 #define TI_RESTART_BLOCK 0x00000498
-#define TI_KUNA_REGS	0x000004c0
-#define TI_KUNA_INSN	0x000004c8
+#define TI_KUNA_REGS	0x000004c8
+#define TI_KUNA_INSN	0x000004d0
 #define TI_FPREGS	0x00000500
 
 /* We embed this in the uppermost byte of thread_info->flags */

Re: [tip:core/futexes] sparc64: extend TI_RESTART_BLOCK space by 8 bytes

[Darren Hart]

Stephen Rothwell wrote:
> Commit-ID:  d2de688891909b148efe83a6fc9520a9cd6015f0
> Gitweb:     http://git.kernel.org/tip/d2de688891909b148efe83a6fc9520a9cd6015f0
> Author:     Stephen Rothwell <sfr@canb.auug.org.au>
> AuthorDate: Thu, 9 Apr 2009 15:17:22 +1000
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Thu, 9 Apr 2009 08:09:50 +0200
> 
> sparc64: extend TI_RESTART_BLOCK space by 8 bytes
> 
> Impact: build fix
> 
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
>   arch/sparc/kernel/built-in.o: In function `trap_init':
>   (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'
> 
> Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
> requeue_pi functionality") (from the tip-core tree) which changed the
> size of struct restart_block.
> 

In order to avoid such situations in the future, perhaps it would make 
sense to add a comment in thread_info.h stating spar64's dependency on 
the size of the restart_block?

Also, I noticed the following embedded in the definition of the 
structure, does enabling this option also break sparc64 builds?

#ifdef CONFIG_COMPAT
			struct compat_timespec __user *compat_rmtp;
#endif

Thanks,

Darren Hart

> Shift TI_KUNA_REGS and TI_KUNA_INSN up by 8 bytes to make space for the
> larger restart block.
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Acked-by: "David S. Miller" <davem@davemloft.net>
> Cc: Darren Hart <dvhltc@us.ibm.com>
> LKML-Reference: <20090409151722.c8eabb56.sfr@canb.auug.org.au>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  arch/sparc/include/asm/thread_info_64.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/thread_info_64.h b/arch/sparc/include/asm/thread_info_64.h
> index 639ac80..6586572 100644
> --- a/arch/sparc/include/asm/thread_info_64.h
> +++ b/arch/sparc/include/asm/thread_info_64.h
> @@ -102,8 +102,8 @@ struct thread_info {
>  #define TI_KERN_CNTD1	0x00000488
>  #define TI_PCR		0x00000490
>  #define TI_RESTART_BLOCK 0x00000498
> -#define TI_KUNA_REGS	0x000004c0
> -#define TI_KUNA_INSN	0x000004c8
> +#define TI_KUNA_REGS	0x000004c8
> +#define TI_KUNA_INSN	0x000004d0
>  #define TI_FPREGS	0x00000500
> 
>  /* We embed this in the uppermost byte of thread_info->flags */


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

Re: [tip:core/futexes] sparc64: extend TI_RESTART_BLOCK space by 8 bytes

[Stephen Rothwell]

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

Hi Darren,

On Thu, 09 Apr 2009 08:23:09 -0700 Darren Hart <dvhltc@us.ibm.com> wrote:
>
> Also, I noticed the following embedded in the definition of the 
> structure, does enabling this option also break sparc64 builds?
> 
> #ifdef CONFIG_COMPAT
> 			struct compat_timespec __user *compat_rmtp;
> #endif

Sparc64 would always have CONFIG_COMPAT enabled.

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

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

Re: [tip:core/futexes] sparc64: extend TI_RESTART_BLOCK space by 8 bytes

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 10 Apr 2009 08:32:09 +1000

> Hi Darren,
> 
> On Thu, 09 Apr 2009 08:23:09 -0700 Darren Hart <dvhltc@us.ibm.com> wrote:
>>
>> Also, I noticed the following embedded in the definition of the 
>> structure, does enabling this option also break sparc64 builds?
>> 
>> #ifdef CONFIG_COMPAT
>> 			struct compat_timespec __user *compat_rmtp;
>> #endif
> 
> Sparc64 would always have CONFIG_COMPAT enabled.

I can be turned off, and people have done this who want to
run a 64-bit only system.

Re: linux-next: sparc tree build failure

[Stephen Rothwell]

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

Hi all,

On Thu, 9 Apr 2009 15:17:22 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next build (sparc64 defconfig) failed like this:
> 
> arch/sparc/kernel/built-in.o: In function `trap_init':
> (.init.text+0x4): undefined reference to `thread_info_offsets_are_bolixed_dave'
> 
> Caused by commit 52400ba946759af28442dee6265c5c0180ac7122 ("futex: add
> requeue_pi functionality") (from the tip-core tree) which changed the
> size of struct restart_block.

This is still happening except the actual error has changed (due to the new BUILD_BUG_ON):

arch/sparc/kernel/traps_64.c: In function 'trap_init':
arch/sparc/kernel/traps_64.c:2534: error: size of array 'type name' is negative

I applied the same patch.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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