LKML Archive on lore.kernel.org
 help / color / Atom feed
* Commit 085219f79cad broke Sparc-32 back in 2.6.28.
       [not found]   ` <201002201712.23628.rob@landley.net>
@ 2010-02-21 16:25     ` Rob Landley
  2010-02-21 23:57       ` David Miller
  2010-02-22  2:06       ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: Rob Landley @ 2010-02-21 16:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Blue Swirl, Paolo Bonzini, Artyom Tarasenko, linux-kernel, sam, davem

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 <sam@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 <sam@ravnborg.org>
    Signed-off-by: David S. Miller <davem@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-02-21 16:25     ` Commit 085219f79cad broke Sparc-32 back in 2.6.28 Rob Landley
@ 2010-02-21 23:57       ` David Miller
  2010-02-22  0:28         ` Bartlomiej Zolnierkiewicz
  2010-02-22  2:06       ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: David Miller @ 2010-02-21 23:57 UTC (permalink / raw)
  To: rob; +Cc: qemu-devel, blauwirbel, pbonzini, atar4qemu, linux-kernel, sam

From: Rob Landley <rob@landley.net>
Date: Sun, 21 Feb 2010 10:25:09 -0600

> 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> Author: Sam Ravnborg <sam@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 <sam@ravnborg.org>
>     Signed-off-by: David S. Miller <davem@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-02-21 23:57       ` David Miller
@ 2010-02-22  0:28         ` Bartlomiej Zolnierkiewicz
  2010-02-22  2:03           ` Rob Landley
  0 siblings, 1 reply; 9+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2010-02-22  0:28 UTC (permalink / raw)
  To: David Miller
  Cc: rob, qemu-devel, blauwirbel, pbonzini, atar4qemu, linux-kernel, sam

On Monday 22 February 2010 12:57:19 am David Miller wrote:
> From: Rob Landley <rob@landley.net>
> Date: Sun, 21 Feb 2010 10:25:09 -0600
> 
> > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > Author: Sam Ravnborg <sam@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 <sam@ravnborg.org>
> >     Signed-off-by: David S. Miller <davem@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-02-22  0:28         ` Bartlomiej Zolnierkiewicz
@ 2010-02-22  2:03           ` Rob Landley
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Landley @ 2010-02-22  2:03 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: David Miller, qemu-devel, blauwirbel, pbonzini, atar4qemu,
	linux-kernel, sam

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 <rob@landley.net>
> > Date: Sun, 21 Feb 2010 10:25:09 -0600
> >
> > > 085219f79cad89291699bd2bfb21c9fdabafe65f is first bad commit
> > > commit 085219f79cad89291699bd2bfb21c9fdabafe65f
> > > Author: Sam Ravnborg <sam@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 <sam@ravnborg.org>
> > >     Signed-off-by: David S. Miller <davem@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-02-21 16:25     ` Commit 085219f79cad broke Sparc-32 back in 2.6.28 Rob Landley
  2010-02-21 23:57       ` David Miller
@ 2010-02-22  2:06       ` David Miller
  2010-03-27  3:35         ` Rob Landley
  1 sibling, 1 reply; 9+ messages in thread
From: David Miller @ 2010-02-22  2:06 UTC (permalink / raw)
  To: rob; +Cc: qemu-devel, blauwirbel, pbonzini, atar4qemu, linux-kernel, sam


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 <rob@landley.net>
Signed-off-by: David S. Miller <davem@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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-02-22  2:06       ` David Miller
@ 2010-03-27  3:35         ` Rob Landley
  2010-03-27  3:37           ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Landley @ 2010-03-27  3:35 UTC (permalink / raw)
  To: David Miller, blauwirbel, linux-kernel

On Sunday 21 February 2010 20:06:58 David Miller wrote:
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-03-27  3:35         ` Rob Landley
@ 2010-03-27  3:37           ` David Miller
  2010-03-27  7:44             ` Rob Landley
  0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2010-03-27  3:37 UTC (permalink / raw)
  To: rob; +Cc: blauwirbel, linux-kernel

From: Rob Landley <rob@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-03-27  3:37           ` David Miller
@ 2010-03-27  7:44             ` Rob Landley
  2010-03-27 23:31               ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Landley @ 2010-03-27  7:44 UTC (permalink / raw)
  To: David Miller; +Cc: blauwirbel, linux-kernel

On Friday 26 March 2010 22:37:45 David Miller wrote:
> From: Rob Landley <rob@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 <rob@landley.net>

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Commit 085219f79cad broke Sparc-32 back in 2.6.28.
  2010-03-27  7:44             ` Rob Landley
@ 2010-03-27 23:31               ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2010-03-27 23:31 UTC (permalink / raw)
  To: rob; +Cc: blauwirbel, linux-kernel

From: Rob Landley <rob@landley.net>
Date: Sat, 27 Mar 2010 02:44:16 -0500

> On Friday 26 March 2010 22:37:45 David Miller wrote:
>> From: Rob Landley <rob@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!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <201002110520.07620.rob@landley.net>
     [not found] ` <f43fc5581002201359x50aff2b1ofc4f816762bda597@mail.gmail.com>
     [not found]   ` <201002201712.23628.rob@landley.net>
2010-02-21 16:25     ` Commit 085219f79cad broke Sparc-32 back in 2.6.28 Rob Landley
2010-02-21 23:57       ` David Miller
2010-02-22  0:28         ` Bartlomiej Zolnierkiewicz
2010-02-22  2:03           ` Rob Landley
2010-02-22  2:06       ` David Miller
2010-03-27  3:35         ` Rob Landley
2010-03-27  3:37           ` David Miller
2010-03-27  7:44             ` Rob Landley
2010-03-27 23:31               ` David Miller

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git