linux-next: build failure after merge of the final tree

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

arch/powerpc/platforms/cell/spufs/inode.c: In function 'spufs_parse_options':
arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible types when assigning to type 'kuid_t' from type 'int'
    root->i_uid = option;
                ^
arch/powerpc/platforms/cell/spufs/inode.c:628:16: error: incompatible types when assigning to type 'kgid_t' from type 'int'
    root->i_gid = option;
                ^

Caused by commit d6970d4b726c ("enable building user namespace with xfs")
from the xfs tree (that was fun to find :-)).

I have reverted that commit for today.  It could probably be replaced
with a patch that just changed XFS_FS to SPU_FS in the UIDGID_CONVERTED
config dependency - or someone could fix up SPU_FS.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Dwight Engen]

On Tue, 20 Aug 2013 17:20:52 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> arch/powerpc/platforms/cell/spufs/inode.c: In function
> 'spufs_parse_options':
> arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible
> types when assigning to type 'kuid_t' from type 'int' root->i_uid =
> option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error:
> incompatible types when assigning to type 'kgid_t' from type 'int'
> root->i_gid = option; ^
> 
> Caused by commit d6970d4b726c ("enable building user namespace with
> xfs") from the xfs tree (that was fun to find :-)).
> 
> I have reverted that commit for today.  It could probably be replaced
> with a patch that just changed XFS_FS to SPU_FS in the
> UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS.

Hi, (already sent this email based on Intel's kbuild robot this
morning, sorry for the dup to those who already got it). 

Yep this looks to me like SPU_FS should have been in the list of
stuff that had not been UIDGID_CONVERTED, but reviving
UIDGID_CONVERTED and adding SPU_FS to it won't work for
non powerpc arch because SPU_FS = n won't be defined. The following can
be used to mark it as incompatible with USER_NS:

diff --git a/arch/powerpc/platforms/cell/Kconfig
b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644
--- a/arch/powerpc/platforms/cell/Kconfig
+++ b/arch/powerpc/platforms/cell/Kconfig
@@ -61,6 +61,7 @@ config SPU_FS
        tristate "SPU file system"
        default m
        depends on PPC_CELL
+       depends on USER_NS=n
        select SPU_BASE
        select MEMORY_HOTPLUG
        help

Or if the rest of spufs is already okay for user namespace (I have not
checked it, but this seems to be the only place it is dealing with
uid/gid), then the following will fix these particular errors
(cross-compile tested, but I don't have a powerpc to run test on):

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index f390042..90fb308 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
                case Opt_uid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_uid = option;
+                       root->i_uid = make_kuid(&init_user_ns, option);
                        break;
                case Opt_gid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_gid = option;
+                       root->i_gid = make_kgid(&init_user_ns, option);
                        break;
                case Opt_mode:
                        if (match_octal(&args[0], &option))

Re: linux-next: build failure after merge of the final tree

[Ben Myers]

Hi Jeremy,

Apologies for breaking your build...

On Tue, Aug 20, 2013 at 12:07:02PM -0400, Dwight Engen wrote:
> On Tue, 20 Aug 2013 17:20:52 +1000
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > After merging the final tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> > 
> > arch/powerpc/platforms/cell/spufs/inode.c: In function
> > 'spufs_parse_options':
> > arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible
> > types when assigning to type 'kuid_t' from type 'int' root->i_uid =
> > option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error:
> > incompatible types when assigning to type 'kgid_t' from type 'int'
> > root->i_gid = option; ^
> > 
> > Caused by commit d6970d4b726c ("enable building user namespace with
> > xfs") from the xfs tree (that was fun to find :-)).
> > 
> > I have reverted that commit for today.  It could probably be replaced
> > with a patch that just changed XFS_FS to SPU_FS in the
> > UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS.
> 
> Hi, (already sent this email based on Intel's kbuild robot this
> morning, sorry for the dup to those who already got it). 
> 
> Yep this looks to me like SPU_FS should have been in the list of
> stuff that had not been UIDGID_CONVERTED, but reviving
> UIDGID_CONVERTED and adding SPU_FS to it won't work for
> non powerpc arch because SPU_FS = n won't be defined. The following can
> be used to mark it as incompatible with USER_NS:
> 
> diff --git a/arch/powerpc/platforms/cell/Kconfig
> b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644
> --- a/arch/powerpc/platforms/cell/Kconfig
> +++ b/arch/powerpc/platforms/cell/Kconfig
> @@ -61,6 +61,7 @@ config SPU_FS
>         tristate "SPU file system"
>         default m
>         depends on PPC_CELL
> +       depends on USER_NS=n
>         select SPU_BASE
>         select MEMORY_HOTPLUG
>         help
> 
> Or if the rest of spufs is already okay for user namespace (I have not
> checked it, but this seems to be the only place it is dealing with
> uid/gid), then the following will fix these particular errors
> (cross-compile tested, but I don't have a powerpc to run test on):
> 
> diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
> index f390042..90fb308 100644
> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
>                 case Opt_uid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_uid = option;
> +                       root->i_uid = make_kuid(&init_user_ns, option);
>                         break;
>                 case Opt_gid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_gid = option;
> +                       root->i_gid = make_kgid(&init_user_ns, option);
>                         break;
>                 case Opt_mode:
>                         if (match_octal(&args[0], &option))

I'd prefer not to break Stephen's tree two days in a row.  We could just revert
d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
the choice would prefer the fix.  Do you have a preference between the two
approaches that Dwight has posted?  The first seems more conservative...

Thanks,
	Ben

Re: linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi Ben,

On Tue, 20 Aug 2013 14:28:44 -0500 Ben Myers <bpm@sgi.com> wrote:
>
> I'd prefer not to break Stephen's tree two days in a row.  We could just revert
> d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
> the choice would prefer the fix.  Do you have a preference between the two
> approaches that Dwight has posted?  The first seems more conservative...

I will automatically revert that commit when I merge the xfs tree until
some other solution is forthcoming (so you don't have to do the revert in
the xfs tree).  This does need to be fixed fairly soon, though.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Ben Myers]

Hey Stephen,

On Wed, Aug 21, 2013 at 10:22:46AM +1000, Stephen Rothwell wrote:
> On Tue, 20 Aug 2013 14:28:44 -0500 Ben Myers <bpm@sgi.com> wrote:
> > I'd prefer not to break Stephen's tree two days in a row.  We could just revert
> > d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
> > the choice would prefer the fix.  Do you have a preference between the two
> > approaches that Dwight has posted?  The first seems more conservative...
> 
> I will automatically revert that commit when I merge the xfs tree until
> some other solution is forthcoming (so you don't have to do the revert in
> the xfs tree).

Gah.  That makes sense.  ;)

> This does need to be fixed fairly soon, though.

Agreed, thanks.

-Ben

Re: linux-next: build failure after merge of the final tree

[Arnd Bergmann]

On Tuesday 20 August 2013, Dwight Engen wrote:
> diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
> index f390042..90fb308 100644
> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
>                 case Opt_uid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_uid = option;
> +                       root->i_uid = make_kuid(&init_user_ns, option);
>                         break;
>                 case Opt_gid:
>                         if (match_int(&args[0], &option))
>                                 return 0;
> -                       root->i_gid = option;
> +                       root->i_gid = make_kgid(&init_user_ns, option);
>                         break;
>                 case Opt_mode:
>                         if (match_octal(&args[0], &option))

Doesn't this mean the uid/gid is taken from the initial namespace rather than
from the namespace of the 'mount' process calling this? I think the logical
choice would be to have the UID be the one that gets passed here in the caller's
namespace.

	Arnd

Re: linux-next: build failure after merge of the final tree

[Dwight Engen]

On Tue, 20 Aug 2013 22:46:30 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 20 August 2013, Dwight Engen wrote:
> > diff --git a/arch/powerpc/platforms/cell/spufs/inode.c
> > b/arch/powerpc/platforms/cell/spufs/inode.c index f390042..90fb308
> > 100644 --- a/arch/powerpc/platforms/cell/spufs/inode.c
> > +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> > @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb,
> > char *options, struct inode *root) case Opt_uid:
> >                         if (match_int(&args[0], &option))
> >                                 return 0;
> > -                       root->i_uid = option;
> > +                       root->i_uid = make_kuid(&init_user_ns,
> > option); break;
> >                 case Opt_gid:
> >                         if (match_int(&args[0], &option))
> >                                 return 0;
> > -                       root->i_gid = option;
> > +                       root->i_gid = make_kgid(&init_user_ns,
> > option); break;
> >                 case Opt_mode:
> >                         if (match_octal(&args[0], &option))
> 
> Doesn't this mean the uid/gid is taken from the initial namespace
> rather than from the namespace of the 'mount' process calling this? I
> think the logical choice would be to have the UID be the one that
> gets passed here in the caller's namespace.

Yes, I agree. The other filesystems that take an Opt_uid as well do use
current_user_ns() and not init_user_ns. They also do a uid_valid()
check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think
that would look like this:

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index f390042..87ba7cf 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -620,12 +620,16 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
                case Opt_uid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_uid = option;
+                       root->i_uid = make_kuid(current_user_ns(), option);
+                       if (!uid_valid(root->i_uid))
+                               return 0;
                        break;
                case Opt_gid:
                        if (match_int(&args[0], &option))
                                return 0;
-                       root->i_gid = option;
+                       root->i_gid = make_kgid(current_user_ns(), option);
+                       if (!gid_valid(root->i_gid))
+                               return 0;
                        break;
                case Opt_mode:
                        if (match_octal(&args[0], &option))

Again, I have not run tested this so we may just want to disable SPU_FS
with USER_NS until they can be tested together.

Re: linux-next: build failure after merge of the final tree

[Jeremy Kerr]

Hi all,

> Yes, I agree. The other filesystems that take an Opt_uid as well do use
> current_user_ns() and not init_user_ns. They also do a uid_valid()
> check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think
> that would look like this:

Looks good to me. Builds and mounts as expected.

Acked-by: Jeremy Kerr <jk@ozlabs.org>

Cheers,


Jeremy

Re: linux-next: build failure after merge of the final tree

[Ben Myers]

Hey Dwight,

On Wed, Aug 21, 2013 at 02:30:04PM +0800, Jeremy Kerr wrote:
> > Yes, I agree. The other filesystems that take an Opt_uid as well do use
> > current_user_ns() and not init_user_ns. They also do a uid_valid()
> > check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think
> > that would look like this:
> 
> Looks good to me. Builds and mounts as expected.
> 
> Acked-by: Jeremy Kerr <jk@ozlabs.org>

Could you repost this patch with the right subject and a commit header?  Given
Jeremy's Ack I think we could proceed to pull this in.

Regards,
	Ben

[PATCH] powerpc/spufs: convert userns uid/gid mount options to kuid/kgid

[Dwight Engen]

Acked-by: Jeremy Kerr <jk@ozlabs.org>
Tested-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
---
 arch/powerpc/platforms/cell/spufs/inode.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index f390042..87ba7cf 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -620,12 +620,16 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
 		case Opt_uid:
 			if (match_int(&args[0], &option))
 				return 0;
-			root->i_uid = option;
+			root->i_uid = make_kuid(current_user_ns(), option);
+			if (!uid_valid(root->i_uid))
+				return 0;
 			break;
 		case Opt_gid:
 			if (match_int(&args[0], &option))
 				return 0;
-			root->i_gid = option;
+			root->i_gid = make_kgid(current_user_ns(), option);
+			if (!gid_valid(root->i_gid))
+				return 0;
 			break;
 		case Opt_mode:
 			if (match_octal(&args[0], &option))
-- 
1.8.1.4

On Wed, 21 Aug 2013 10:56:54 -0500
Ben Myers <bpm@sgi.com> wrote:

> Hey Dwight,
> 
> On Wed, Aug 21, 2013 at 02:30:04PM +0800, Jeremy Kerr wrote:
> > > Yes, I agree. The other filesystems that take an Opt_uid as well
> > > do use current_user_ns() and not init_user_ns. They also do a
> > > uid_valid() check and fail the mount (or fallback to
> > > GLOBAL_ROOT_UID). So I think that would look like this:
> > 
> > Looks good to me. Builds and mounts as expected.
> > 
> > Acked-by: Jeremy Kerr <jk@ozlabs.org>
> 
> Could you repost this patch with the right subject and a commit
> header?  Given Jeremy's Ack I think we could proceed to pull this in.

Sure, I just wanted to make sure someone had tested it first, which it
looks like Jeremy did, thanks.

> Regards,
> 	Ben

Re: [PATCH] powerpc/spufs: convert userns uid/gid mount options to kuid/kgid

[Arnd Bergmann]

On Wednesday 21 August 2013, Dwight Engen wrote:
> 
> Acked-by: Jeremy Kerr <jk@ozlabs.org>
> Tested-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] powerpc/spufs: convert userns uid/gid mount options to kuid/kgid

[Ben Myers]

On Wed, Aug 21, 2013 at 10:05:27PM +0200, Arnd Bergmann wrote:
> On Wednesday 21 August 2013, Dwight Engen wrote:
> > 
> > Acked-by: Jeremy Kerr <jk@ozlabs.org>
> > Tested-by: Jeremy Kerr <jk@ozlabs.org>
> > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> 
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Applied.

Thanks,
	-Ben

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
arch/powerpc/kernel/exceptions-64s.S:1312: Error: attempt to move .org backwards

The last time I got this error, I needed to apply patch "powerpc: Fix
"attempt to move .org backwards" error", but that has been included in
the powerpc tree now, so I guess something else has added code in a
critical place. :-(

I have just left this broken for today.
-- 
Cheers,
Stephen Rothwell <sfr@canb.auug.org.au>

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

Re: linux-next: build failure after merge of the final tree

[Benjamin Herrenschmidt]

On Mon, 2014-01-06 at 20:28 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
> arch/powerpc/kernel/exceptions-64s.S:1312: Error: attempt to move .org backwards
> 
> The last time I got this error, I needed to apply patch "powerpc: Fix
> "attempt to move .org backwards" error", but that has been included in
> the powerpc tree now, so I guess something else has added code in a
> critical place. :-(
> 
> I have just left this broken for today.

I had to modify that patch when applying it, it's possible that the
"new" version isn't making as much room. Without that change it would
fail the build on some of my configs due to some of the asm for the
maskable exception handling being too far from the conditional branches
that calls it.

I think it's time we do a bit of re-org of that file to figure out
precisely what has to be where and move things out more aggressively.

Cheers,
Ben.

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

powerpc64-linux-ld: drivers/built-in.o: In function `.gpiochip_is_requested':
(.text+0x4): sibling call optimization to `_savegpr0_29' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `_savegpr0_29' extern

I got more than 60000 of these messages before I killed the link. :-(  I
am not sure what has changed to do this, but it may have been masked for
the past few releases due to other linking problems.

I have left this broken for today.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Alan Modra]

On Thu, Jul 05, 2012 at 06:33:45PM +1000, Stephen Rothwell wrote:
> powerpc64-linux-ld: drivers/built-in.o: In function `.gpiochip_is_requested':
> (.text+0x4): sibling call optimization to `_savegpr0_29' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `_savegpr0_29' extern
> 
> I got more than 60000 of these messages before I killed the link. :-(  I
> am not sure what has changed to do this, but it may have been masked for
> the past few releases due to other linking problems.

Let me guess.  You're using bleeding edge gcc but not binutils.

a) Recent gcc has fixed prologue and epilogue generation which now
   properly makes use of out-of-line register save and restore
   functions when compiling with -Os.
b) Recent ld doesn't emit out-of-line save/restore function for ld -r,
   but yours does.  You need my 2012-06-22 patch.
c) Kernel uses ld -r for packaging.

(b) and (c) together mean you get a definition for _savegpr0_29 munged
together with other functions.  That's bad.  If _savegpr0_29 wasn't
emitted until the final link stage then it would be in a code section
containing just save/restore functions.  ld will analyse that section
and notice the absense of toc relocations; functions therein don't
use the toc and can thus be called from any toc group without needing
a toc adjusting stub.  In your case _savegpr0_29 is in a section that
has toc relocations (from normal compiled code), so ld decides that
any function in that section must have a proper value for the toc
register.  But calls to _savegpr0_29 don't have a following nop to
overwrite with a toc restore insn, hence the ld error.

Score another black mark for ld -r.

-- 
Alan Modra
Australia Development Lab, IBM

Re: linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi Alan,

On Thu, 5 Jul 2012 19:13:48 +0930 Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Jul 05, 2012 at 06:33:45PM +1000, Stephen Rothwell wrote:
> > powerpc64-linux-ld: drivers/built-in.o: In function `.gpiochip_is_requested':
> > (.text+0x4): sibling call optimization to `_savegpr0_29' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `_savegpr0_29' extern
> > 
> > I got more than 60000 of these messages before I killed the link. :-(  I
> > am not sure what has changed to do this, but it may have been masked for
> > the past few releases due to other linking problems.
> 
> Let me guess.  You're using bleeding edge gcc but not binutils.

powerpc-linux-gcc (GCC) 4.6.3
GNU ld (GNU Binutils) 2.22

both built from upstream sources (by Tony).

> a) Recent gcc has fixed prologue and epilogue generation which now
>    properly makes use of out-of-line register save and restore
>    functions when compiling with -Os.
> b) Recent ld doesn't emit out-of-line save/restore function for ld -r,
>    but yours does.  You need my 2012-06-22 patch.
> c) Kernel uses ld -r for packaging.
> 
> (b) and (c) together mean you get a definition for _savegpr0_29 munged
> together with other functions.  That's bad.  If _savegpr0_29 wasn't
> emitted until the final link stage then it would be in a code section
> containing just save/restore functions.  ld will analyse that section
> and notice the absense of toc relocations; functions therein don't
> use the toc and can thus be called from any toc group without needing
> a toc adjusting stub.  In your case _savegpr0_29 is in a section that
> has toc relocations (from normal compiled code), so ld decides that
> any function in that section must have a proper value for the toc
> register.  But calls to _savegpr0_29 don't have a following nop to
> overwrite with a toc restore insn, hence the ld error.
> 
> Score another black mark for ld -r.

OK, the new toolchain may be the problem.  I changed from:

powerpc-linux-gcc (GCC) 4.6.0
GNU ld (GNU Binutils) 2.21

on June 20 and the current errors may have been masked an early bailout
after getting theses errors:

powerpc64-linux-ld: arch/powerpc/net/built-in.o: In function `bpf_slow_path_word':
(.text+0x90): sibling call optimization to `skb_copy_bits' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `skb_copy_bits' extern

which have now been fixed.  So would a simple patch that puts the
_savegpr etc functions in their own section (defined how?) fix this for
us?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Alan Modra]

On Fri, Jul 06, 2012 at 10:21:51AM +1000, Stephen Rothwell wrote:
> which have now been fixed.  So would a simple patch that puts the
> _savegpr etc functions in their own section (defined how?) fix this for
> us?

Ah, the kernel provides its own save/restore functions, and these get
mashed into a .text containing normal functions with toc references by
ld -r.  Well, you could stop using ld -r.  Otherwise, try

 .section ".text.save.restore","ax",@progbits

-- 
Alan Modra
Australia Development Lab, IBM

Re: linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi Alan,

On Fri, 6 Jul 2012 10:27:10 +0930 Alan Modra <amodra@gmail.com> wrote:
>
> On Fri, Jul 06, 2012 at 10:21:51AM +1000, Stephen Rothwell wrote:
> > which have now been fixed.  So would a simple patch that puts the
> > _savegpr etc functions in their own section (defined how?) fix this for
> > us?
> 
> Ah, the kernel provides its own save/restore functions, and these get
> mashed into a .text containing normal functions with toc references by
> ld -r.  Well, you could stop using ld -r.  Otherwise, try
> 
>  .section ".text.save.restore","ax",@progbits

OK, so that helped a lot.  Now I get this:

drivers/built-in.o: In function `.idt77252_init_ubr.isra.10':
idt77252.c:(.text+0x1ff801c): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.idt77252_init_tx':
idt77252.c:(.text+0x1ff8258): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.idt77252_change_qos':
idt77252.c:(.text+0x1ff86a0): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.idt77252_open':
idt77252.c:(.text+0x1ff898c): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
idt77252.c:(.text+0x1ff8f4c): relocation truncated to fit: R_PPC64_REL24 against symbol `_restgpr0_22' defined in .text.save.restore section in arch/powerpc/lib/built-in.o
drivers/built-in.o: In function `.next_string':
solos-pci.c:(.text+0x1ff8f54): relocation truncated to fit: R_PPC64_REL24 against symbol `_savegpr0_29' defined in .text.save.restore section in arch/powerpc/lib/built-in.o
solos-pci.c:(.text+0x1ff8f64): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.print_buffer':
solos-pci.c:(.text+0x1ff904c): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.atm_remove':
solos-pci.c:(.text+0x1ff91d4): relocation truncated to fit: R_PPC64_REL24 against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
solos-pci.c:(.text+0x1ff923c): relocation truncated to fit: R_PPC64_REL24 against symbol `.sysfs_remove_group' defined in .text section in fs/built-in.o
drivers/built-in.o: In function `.fpga_tx':
solos-pci.c:(.text+0x1ff950c): additional relocation overflows omitted from the output

I assume at this point, we are just too large.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Alan Modra]

On Fri, Jul 06, 2012 at 01:01:37PM +1000, Stephen Rothwell wrote:
> solos-pci.c:(.text+0x1ff923c): relocation truncated to fit: R_PPC64_REL24
                     ^^^^^^^^^

> I assume at this point, we are just too large.

Yeah, but not in total.  I didn't see any of these in the allyes
kernel I built with our proof of concept hack to avoid ld -r.  I think
you'll find that these are all from ld -r output, as I assume no one
in kernel land writes drivers or whatever with 33M of text in a single
file.  Branches in that monstrous section can't even reach the
trampolines that ld inserts to extend branch reach.  Did I mention
that ld -r is a bad idea?

One workaround might be to compile with -ffunction-sections.

-- 
Alan Modra
Australia Development Lab, IBM

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
ppc44x_defconfig) failed like this:

arch/powerpc/kernel/pci-common.c: In function 'pcibios_setup_phb_resources':
arch/powerpc/kernel/pci-common.c:1520:4: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]

Caused by commit 6c5705fec63d ("powerpc/PCI: get rid of device resource
fixups") from the pci tree.  In this build, resource_size_t is 64 bits
while pointers are only 32.

I applied the following fix patch.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 27 Feb 2012 17:33:48 +1100
Subject: [PATCH] powerpc/PCI: fix up for mismatch between resource_size_t and
 pointer size

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/kernel/pci-common.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 910b9de..808ecbb 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1517,7 +1517,7 @@ static void __devinit pcibios_setup_phb_resources(struct pci_controller *hose, s
 		 (unsigned long long)res->end,
 		 (unsigned long)res->flags);
 	pci_add_resource_offset(resources, res,
-			(resource_size_t) hose->io_base_virt - _IO_BASE);
+			(resource_size_t)(unsigned long)hose->io_base_virt - _IO_BASE);
 
 	/* Hookup PHB Memory resources */
 	for (i = 0; i < 3; ++i) {
-- 
1.7.9.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Benjamin Herrenschmidt]

On Mon, 2012-02-27 at 17:37 +1100, Stephen Rothwell wrote:
>         pci_add_resource_offset(resources, res,
> -                       (resource_size_t) hose->io_base_virt - _IO_BASE);
> +                       (resource_size_t)(unsigned long)hose->io_base_virt - _IO_BASE);

We have to be careful here as we do want sign extension to happen (yeah
it's odd, but it's the way we do IOs on ppc32 :-) Maybe I should change
it one day).

So we probably want to do:

	 (resource_size_t)(long long)(hose->io_base_virt - _IO_BASE)

Basically, IO resources are relative to _IO_BASE which on ppc32 is
basically the virtual address where we map the first PHB IO space.

Subsequent PHB mappings can end up below _IO_BASE, leading to negative
resource values for IO BARs on those busses. It all works fine because
even an unsigned addition will do the right thing as long as the value
is fully sign extended.

Cheers,
Ben.

Re: linux-next: build failure after merge of the final tree

[Benjamin Herrenschmidt]

On Mon, 2012-02-27 at 20:19 +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2012-02-27 at 17:37 +1100, Stephen Rothwell wrote:
> >         pci_add_resource_offset(resources, res,
> > -                       (resource_size_t) hose->io_base_virt - _IO_BASE);
> > +                       (resource_size_t)(unsigned long)hose->io_base_virt - _IO_BASE);
> 
> We have to be careful here as we do want sign extension to happen (yeah
> it's odd, but it's the way we do IOs on ppc32 :-) Maybe I should change
> it one day).
> 
> So we probably want to do:
> 
> 	 (resource_size_t)(long long)(hose->io_base_virt - _IO_BASE)

Oops ... that was meant to read (long) not (long long)... Any ways, I
more or less convinced myself that even without the sign extension it
would still work, since the IO port number is eventually cast to an
unsigned int by the accessors, so as long as the low 32-bits are correct
(and they'll be with or without the sign extension), we should be fine.
It's just that something trying to print the resource might end up
displaying garbage in the top bits.

Cheers,
Ben.


> Basically, IO resources are relative to _IO_BASE which on ppc32 is
> basically the virtual address where we map the first PHB IO space.
> 
> Subsequent PHB mappings can end up below _IO_BASE, leading to negative
> resource values for IO BARs on those busses. It all works fine because
> even an unsigned addition will do the right thing as long as the value
> is fully sign extended.
> 
> Cheers,
> Ben.

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allmodconfig) failed like this:

arch/powerpc/platforms/pseries/processor_idle.c:35:6: error: redefinition of 'update_smt_snooze_delay'
arch/powerpc/include/asm/system.h:230:20: note: previous definition of 'update_smt_snooze_delay' was here
arch/powerpc/platforms/pseries/processor_idle.c:175:5: error: redefinition of 'pseries_notify_cpuidle_add_cpu'
arch/powerpc/include/asm/system.h:231:19: note: previous definition of 'pseries_notify_cpuidle_add_cpu' was here

Caused by commit 707827f3387d ("powerpc/cpuidle: cpuidle driver for
pSeries").  For this build, CONFIG_PSERIES_IDLE is "m".

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: linux-next: build failure after merge of the final tree

[Deepthi Dharwar]

On 01/20/2012 12:51 PM, Stephen Rothwell wrote:

> Hi all,
> 
> After merging the final tree, today's linux-next build (powerpc
> allmodconfig) failed like this:
> 
> arch/powerpc/platforms/pseries/processor_idle.c:35:6: error: redefinition of 'update_smt_snooze_delay'
> arch/powerpc/include/asm/system.h:230:20: note: previous definition of 'update_smt_snooze_delay' was here
> arch/powerpc/platforms/pseries/processor_idle.c:175:5: error: redefinition of 'pseries_notify_cpuidle_add_cpu'
> arch/powerpc/include/asm/system.h:231:19: note: previous definition of 'pseries_notify_cpuidle_add_cpu' was here
> 
> Caused by commit 707827f3387d ("powerpc/cpuidle: cpuidle driver for
> pSeries").  For this build, CONFIG_PSERIES_IDLE is "m".
> 
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev




Hi Stephen,

We had a discussion on this particular problem on ppcdev list a few
days back and concluded that it is best not have pseries_idle
as a module.

http://old.nabble.com/-PATCH--cpuidle%3A-Default-y-for-pseries-to33118294.html#a33127587

The following patch disables pseries cpuidle driver to be loaded as a
module as there are build problems reported when one is trying to do so.

arch/powerpc/platforms/pseries/processor_idle.c:35:6: error:
redefinition of 'update_smt_snooze_delay'
arch/powerpc/include/asm/system.h:230:20: note:
previous definition of 'update_smt_snooze_delay' was here
arch/powerpc/platforms/pseries/processor_idle.c:175:5:
error: redefinition of 'pseries_notify_cpuidle_add_cpu'
arch/powerpc/include/asm/system.h:231:19: note:
previous definition of 'pseries_notify_cpuidle_add_cpu' was here

Since the above two functions
are used in core power architecture functions (store_smt_snooze_delay
at kernel/sysfs.c and smp_xics_setup_cpu at platforms/pseries/smp.c),
this requires some rework in these interactions. For now please
disable PSERIES_IDLE to be built as a module for now.

Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index ae7b6d4..31f22c1 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -122,7 +122,7 @@ config DTL
 	  Say N if you are unsure.

 config PSERIES_IDLE
-	tristate "Cpuidle driver for pSeries platforms"
+	bool "Cpuidle driver for pSeries platforms"
 	depends on CPU_IDLE
 	depends on PPC_PSERIES
 	default y

Regards,
Deepthi

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allysconfig) failed like this:

arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
arch/powerpc/kernel/exceptions-64s.S:1151: Error: attempt to move .org backwards
arch/powerpc/kernel/exceptions-64s.S:1160: Error: attempt to move .org backwards

This is probably powerpc or kvm tree related.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
arch/powerpc/kernel/exceptions-64s.S:989: Error: attempt to move .org backwards
arch/powerpc/kernel/exceptions-64s.S:999: Error: attempt to move .org backwards
arch/powerpc/kernel/exceptions-64s.S:1008: Error: attempt to move .org backwards

So something has added a bit of bloat in there.  I have left this broken
for now.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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