2010-02-21 16:36:12

by Rob Landley

[permalink] [raw]
Subject: Commit 085219f79cad broke Sparc-32 back in 2.6.28.

On Saturday 20 February 2010 17:12:22 Rob Landley wrote:
> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

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


2010-02-21 23:57:06

by David Miller

[permalink] [raw]
Subject: Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.

From: Rob Landley <[email protected]>
Date: Sun, 21 Feb 2010 10:25:09 -0600

> 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> Author: Sam Ravnborg <[email protected]>
> 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 <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
> 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.

Subject: Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.

On Monday 22 February 2010 12:57:19 am David Miller wrote:
> From: Rob Landley <[email protected]>
> Date: Sun, 21 Feb 2010 10:25:09 -0600
>
> > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > Author: Sam Ravnborg <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: David S. Miller <[email protected]>
> >
> > 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

2010-02-22 02:03:41

by Rob Landley

[permalink] [raw]
Subject: Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.

On Sunday 21 February 2010 18:28:20 Bartlomiej Zolnierkiewicz wrote:
> On Monday 22 February 2010 12:57:19 am David Miller wrote:
> > From: Rob Landley <[email protected]>
> > Date: Sun, 21 Feb 2010 10:25:09 -0600
> >
> > > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > > Author: Sam Ravnborg <[email protected]>
> > > 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 <[email protected]>
> > > Signed-off-by: David S. Miller <[email protected]>
> > >
> > > 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

2010-02-22 02:06:42

by David Miller

[permalink] [raw]
Subject: Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.


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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
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