Commit 085219f79cad broke Sparc-32 back in 2.6.28.
Rob Landley
> On Saturday 20 February 2010 15:59:31 Blue Swirl wrote:
> > > I've got 2.6.32 booting to a command prompt (albeit with serial console
> > > and intentionall restricted set of hardware). But then it misbehaves.
> > >
> > > I'll try getting 2.6.18 to build with a known .config, and then bisect
> > > forward if that seems to work...
> >
> > Good plan. Bisecting backwards could be interesting too, to find out
> > which releases are actually working out of the box.
>
> I started by iterating through the release versions. It's working up
> through 2.6.28, then 2.6.29 has the out of memory error in my init script.
>
> Bisecting now...
>
> Rob
And the commit that broke it bisects to:
085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
commit 085219f79cad89291699bd2bfb21c9fdabafe65f
Author: Sam Ravnborg <s...@ravnborg.org>
Date: Fri Jan 2 18:47:34 2009 -0800
sparc32: use proper types in struct stat
Like sparc64 use proper types in struct stat
Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
Signed-off-by: David S. Miller <da...@davemloft.net>
This commit breaks stat and makes sparc32 essentially unusable. It changes
the size of the various types in stat.h, and means that if you "mount -t tmpfs
/tmp /tmp" and then try to ls /tmp, ls dies with a memory allocation error.
I've confirmed that reverting it fixes the problem.
Looking at the actual diff, here's the hunk that causes problems:
--- a/arch/sparc/include/asm/stat_32.h
+++ b/arch/sparc/include/asm/stat_32.h
short st_nlink;
- unsigned short st_uid;
- unsigned short st_gid;
+ uid_t st_uid;
+ gid_t st_gid;
The symptom (in my uClibc+busybox root filesystem) is:
/ # mount -t tmpfs /tmp /tmp
/ # ls -l /tmp
ls: can't open '/tmp': Cannot allocate memory
total 0
The problem is that both uid_t and gid_t are "int" instead of "short". This
patch changes the size of those types. (I note that this is apparently a
known issue, there's __compat_uid_t and friends in the sparc asm directory...)
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
David Miller
Date: Sun, 21 Feb 2010 10:25:09 -0600
> 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> Author: Sam Ravnborg <s...@ravnborg.org>
> Date: Fri Jan 2 18:47:34 2009 -0800
>
> sparc32: use proper types in struct stat
>
> Like sparc64 use proper types in struct stat
>
> Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
> Signed-off-by: David S. Miller <da...@davemloft.net>
>
> This commit breaks stat and makes sparc32 essentially unusable. It changes
> the size of the various types in stat.h, and means that if you "mount -t tmpfs
> /tmp /tmp" and then try to ls /tmp, ls dies with a memory allocation error.
>
> I've confirmed that reverting it fixes the problem.
Thanks for tracking this down Rob, I'll work on a fix and
push it around.
Bartlomiej Zolnierkiewicz
> From: Rob Landley <r...@landley.net>
> Date: Sun, 21 Feb 2010 10:25:09 -0600
>
> > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > Author: Sam Ravnborg <s...@ravnborg.org>
> > Date: Fri Jan 2 18:47:34 2009 -0800
> >
> > sparc32: use proper types in struct stat
> >
> > Like sparc64 use proper types in struct stat
> >
> > Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
> > Signed-off-by: David S. Miller <da...@davemloft.net>
> >
> > This commit breaks stat and makes sparc32 essentially unusable. It changes
> > the size of the various types in stat.h, and means that if you "mount -t tmpfs
> > /tmp /tmp" and then try to ls /tmp, ls dies with a memory allocation error.
> >
> > I've confirmed that reverting it fixes the problem.
>
> Thanks for tracking this down Rob, I'll work on a fix and
> push it around.
Looking at how whole sparc32 has been apparently broken for over a year now
because of a purely cleanup patch I wonder if it would be appropriate to
make sparc32 into 'legacy only' and provide 'a stability promise' for it?
Just an idea.. ;)
--
Bartlomiej Zolnierkiewicz
Rob Landley
> On Monday 22 February 2010 12:57:19 am David Miller wrote:
> > From: Rob Landley <r...@landley.net>
> > Date: Sun, 21 Feb 2010 10:25:09 -0600
> >
> > > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > > Author: Sam Ravnborg <s...@ravnborg.org>
> > > Date: Fri Jan 2 18:47:34 2009 -0800
> > >
> > > sparc32: use proper types in struct stat
> > >
> > > Like sparc64 use proper types in struct stat
> > >
> > > Signed-off-by: Sam Ravnborg <s...@ravnborg.org>
> > > Signed-off-by: David S. Miller <da...@davemloft.net>
> > >
> > > This commit breaks stat and makes sparc32 essentially unusable. It
> > > changes the size of the various types in stat.h, and means that if you
> > > "mount -t tmpfs /tmp /tmp" and then try to ls /tmp, ls dies with a
> > > memory allocation error.
> > >
> > > I've confirmed that reverting it fixes the problem.
> >
> > Thanks for tracking this down Rob, I'll work on a fix and
> > push it around.
>
> Looking at how whole sparc32 has been apparently broken for over a year now
> because of a purely cleanup patch I wonder if it would be appropriate to
> make sparc32 into 'legacy only' and provide 'a stability promise' for it?
>
> Just an idea.. ;)
Actually, the problem is that lots of people seem to expect current kernels to
be broken on non-x86 targets, so they keep using old versions. (In the case
of the debian release everybody kept pointing me to on "but it works fine!"
grounds, a 2.6.18 kernel.) Lots of them only upgrade once idiots like me have
gone across the minefield and made it safe. :)
"Current is always broken so nobody uses current" != "nobody uses this
platform". More "sparc people use distros rather than building their own
systems from source, and tend not to be aggressive about upgrading".
Back in 2007 arm was broken for me for two or three releases (according to my
blog it broke in 2.6.20 and the patch that fixed it (
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4454/1 ) was
not yet in 2.6.22-rc7. That doesn't mean arm isn't widely used, just that
nobody with that hardware was seriously trying to use the current version of
the kernel.
My Firmware LInux project is working on implementing automated regression
testing under QEMU. Once I've got a platform working (which sparc wasn't
until now) I can provide much more prompt breakage reports in future, at least
for the basic stuff like this...
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
David Miller
Here's the fix I'll use, thanks for the report Rob:
sparc32: Fix struct stat uid/gid types.
Commit 085219f79cad89291699bd2bfb21c9fdabafe65f
("sparc32: use proper types in struct stat")
Accidently changed the struct stat uid/gid members
to uid_t and gid_t, but those get set to
__kernel_uid32_t and __kernel_gid32_t respectively.
Those are of type 'int' but the structure is meant
to have 'short'. So use uid16_t and gid16_t to
correct this.
Reported-by: Rob Landley <r...@landley.net>
Signed-off-by: David S. Miller <da...@davemloft.net>
---
arch/sparc/include/asm/stat.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sparc/include/asm/stat.h b/arch/sparc/include/asm/stat.h
index 55db5ec..39327d6 100644
--- a/arch/sparc/include/asm/stat.h
+++ b/arch/sparc/include/asm/stat.h
@@ -53,8 +53,8 @@ struct stat {
ino_t st_ino;
mode_t st_mode;
short st_nlink;
- uid_t st_uid;
- gid_t st_gid;
+ uid16_t st_uid;
+ gid16_t st_gid;
unsigned short st_rdev;
off_t st_size;
time_t st_atime;
--
1.6.6.1
Rob Landley
> Here's the fix I'll use, thanks for the report Rob:
>
> sparc32: Fix struct stat uid/gid types.
>
> Commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> ("sparc32: use proper types in struct stat")
Unfortunately, while this fix makes sparc buidl and run again, the exported
kernel headers are horked and can't build strace natively.
gcc -DHAVE_CONFIG_H -I. -Ilinux/sparc -I./linux/sparc -Ilinux -I./linux -
Wall --static -MT file.o -MD -MP -MF .deps/file.Tpo -c -o file.o file.c
In file included from file.c:88:
/usr/bin/../include/asm/stat.h:56: error: expected specifier-qualifier-list
before 'uid16_t'
file.c: In function 'realprintstat':
file.c:951: warning: format '%lu' expects type 'long unsigned int', but
argument 2 has type 'unsigned int'
make[1]: *** [file.o] Error 1
make[1]: Leaving directory `/home/strace-4.5.19'
make: *** [all] Error 2
The problem is that uid16_t is a kernel internal type that gets cleaned out of
the headers when they're exported, thus there's no definition for userspace to
pick up if that structure is ever used from a userspace build.
What exactly was the problem with just saying "unsigned short" when you mean
an unsigned short? The way x86 does, and arm? (If these ever change, it
breaks binary compatability. Not sure what these changes were trying to
accomplish...)
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
David Miller
Date: Fri, 26 Mar 2010 22:35:47 -0500
> What exactly was the problem with just saying "unsigned short" when you mean
> an unsigned short? The way x86 does, and arm? (If these ever change, it
> breaks binary compatability. Not sure what these changes were trying to
> accomplish...)
I was trying to use well defined types that described the
usage and the origin of the definition.
I'm happy to use "unsigned short" or whatever works better.
Please send a patch.
Rob Landley
> From: Rob Landley <r...@landley.net>
> Date: Fri, 26 Mar 2010 22:35:47 -0500
>
> > What exactly was the problem with just saying "unsigned short" when you
> > mean an unsigned short? The way x86 does, and arm? (If these ever
> > change, it breaks binary compatability. Not sure what these changes were
> > trying to accomplish...)
>
> I was trying to use well defined types that described the
> usage and the origin of the definition.
>
> I'm happy to use "unsigned short" or whatever works better.
> Please send a patch.
Sure thing:
--- a/arch/sparc/include/asm/stat.h
+++ b/arch/sparc/include/asm/stat.h
@@ -53,8 +53,8 @@ struct stat {
ino_t st_ino;
mode_t st_mode;
short st_nlink;
- uid16_t st_uid;
- gid16_t st_gid;
+ unsigned short st_uid;
+ unsigned short st_gid;
unsigned short st_rdev;
off_t st_size;
time_t st_atime;
Signed-off-by: Rob Landley <r...@landley.net>
Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
David Miller
Date: Sat, 27 Mar 2010 02:44:16 -0500
> On Friday 26 March 2010 22:37:45 David Miller wrote:
>> From: Rob Landley <r...@landley.net>
>> Date: Fri, 26 Mar 2010 22:35:47 -0500
>>
>> > What exactly was the problem with just saying "unsigned short" when you
>> > mean an unsigned short? The way x86 does, and arm? (If these ever
>> > change, it breaks binary compatability. Not sure what these changes were
>> > trying to accomplish...)
>>
>> I was trying to use well defined types that described the
>> usage and the origin of the definition.
>>
>> I'm happy to use "unsigned short" or whatever works better.
>> Please send a patch.
>
> Sure thing:
Thanks a lot Rob, applied.
Please supply a complete commit log message and place your signoff
right before (instead of after) the patch next time, thanks!