public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: Linux 2.6.6-rc3
       [not found] <1PMQ9-5K6-3@gated-at.bofh.it>
@ 2004-04-28 14:48 ` Vincent C Jones
  2004-04-28 16:00   ` Sam Ravnborg
       [not found] ` <1PVTx-4rY-25@gated-at.bofh.it>
  1 sibling, 1 reply; 39+ messages in thread
From: Vincent C Jones @ 2004-04-28 14:48 UTC (permalink / raw)
  To: linux-kernel

In article <1PMQ9-5K6-3@gated-at.bofh.it> you write:
>
>s390, cifs, ntfs, ppc, ppc64, cpufreq upates. Oh, and DVB and USB.
>
>I'm hoping to do a final 2.6.6 later this week, so I'm hoping as many 
>people as possible will test this.
>
>	Thanks,
>
>		Linus

loop-AES 2.0g has refused to compile since 2.6.6-rc1. No problems up
through 2.6.5. The make script cd's to the kernel source root, then
can't find any of the kernel include files, and it's all downhill from
there...

SuSE 9.0 on a ThinkPad X23. I'd be glad to provide more info if it would
help.

Vince
-- 
Dr. Vincent C. Jones, PE              Expert advice and a helping hand
Computer Network Consultant           for those who want to manage and
Networking Unlimited, Inc.            control their networking destiny
14 Dogwood Lane, Tenafly, NJ 07670
http://www.networkingunlimited.com
VCJones@NetworkingUnlimited.com  +1 201 568-7810  Fax: +1 201 568-7269 

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

* Re: Linux 2.6.6-rc3
  2004-04-28 14:48 ` Linux 2.6.6-rc3 Vincent C Jones
@ 2004-04-28 16:00   ` Sam Ravnborg
  2004-04-28 17:24     ` Jari Ruusu
  0 siblings, 1 reply; 39+ messages in thread
From: Sam Ravnborg @ 2004-04-28 16:00 UTC (permalink / raw)
  To: Vincent C Jones; +Cc: linux-kernel

On Wed, Apr 28, 2004 at 10:48:01AM -0400, Vincent C Jones wrote:
> In article <1PMQ9-5K6-3@gated-at.bofh.it> you write:
> >
> >s390, cifs, ntfs, ppc, ppc64, cpufreq upates. Oh, and DVB and USB.
> >
> >I'm hoping to do a final 2.6.6 later this week, so I'm hoping as many 
> >people as possible will test this.
> >
> >	Thanks,
> >
> >		Linus
> 
> loop-AES 2.0g has refused to compile since 2.6.6-rc1. No problems up
> through 2.6.5. The make script cd's to the kernel source root, then
> can't find any of the kernel include files, and it's all downhill from
> there...

This is an external module - right?
Could you mail me privately the Makefile used, or even better
the full package (or link to it).

	Sam

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

* Re: Linux 2.6.6-rc3
  2004-04-28 16:00   ` Sam Ravnborg
@ 2004-04-28 17:24     ` Jari Ruusu
  2004-04-28 19:08       ` Sam Ravnborg
  2004-04-28 19:18       ` Vincent C Jones
  0 siblings, 2 replies; 39+ messages in thread
From: Jari Ruusu @ 2004-04-28 17:24 UTC (permalink / raw)
  To: Sam Ravnborg, Vincent C Jones; +Cc: linux-kernel

Sam Ravnborg wrote:
> On Wed, Apr 28, 2004 at 10:48:01AM -0400, Vincent C Jones wrote:
> > In article <1PMQ9-5K6-3@gated-at.bofh.it> you write:
> > >
> > >s390, cifs, ntfs, ppc, ppc64, cpufreq upates. Oh, and DVB and USB.
> > >
> > >I'm hoping to do a final 2.6.6 later this week, so I'm hoping as many
> > >people as possible will test this.
> > >
> > >     Thanks,
> > >
> > >             Linus
> >
> > loop-AES 2.0g has refused to compile since 2.6.6-rc1. No problems up
> > through 2.6.5. The make script cd's to the kernel source root, then
> > can't find any of the kernel include files, and it's all downhill from
> > there...
> 
> This is an external module - right?
> Could you mail me privately the Makefile used, or even better
> the full package (or link to it).

Full package:
http://loop-aes.sourceforge.net/loop-AES/loop-AES-v2.0g.tar.bz2

Patch to fix build problem:
http://loop-aes.sourceforge.net/updates/loop-AES-v2.0g-20040412.diff.bz2

New version of loop-AES will be released shortly after final 2.6.6 kernel is
released.

-- 
Jari Ruusu  1024R/3A220F51 5B 4B F9 BB D3 3F 52 E9  DB 1D EB E3 24 0E A9 DD

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

* Re: Linux 2.6.6-rc3
  2004-04-28 17:24     ` Jari Ruusu
@ 2004-04-28 19:08       ` Sam Ravnborg
  2004-04-28 19:18       ` Vincent C Jones
  1 sibling, 0 replies; 39+ messages in thread
From: Sam Ravnborg @ 2004-04-28 19:08 UTC (permalink / raw)
  To: Jari Ruusu; +Cc: Sam Ravnborg, Vincent C Jones, linux-kernel

> 
> Full package:
> http://loop-aes.sourceforge.net/loop-AES/loop-AES-v2.0g.tar.bz2

Hi Jari.
I took a look at how you use the build system in the 2.6 kernel.
Inherited from the 2.4 days you assemble the commands yourself,
which is plain wrong in a 2.6 kernel.
The only sane way to build external modules with a 2.6 kernel is
to utilise the kbuild infrastructure.

For your reference here is a Makefile that I used to compile your
module (made a symling for loop-patched.c file).

To compile the kernel I used:
make -C /home/sam/bk/v2.6/ M=$PWD
[Assumes latest linus kernel - 2.6.6-rc3]

For older kernels append the modules target

This has the added benefit that Module versioning is also supported.

#########################
# Minimal kbuild Makefile for loop-AES

EXTRA_CFLAGS := $(LOOP_AES_CFLAGS)

obj-m := loop.o

i586-$(CONFIG_M586) := -i586
i586-$(CONFIG_M686) := -i586

loop-y := loop-patched.o aes$(i586-y).o glue.o md5$(i586-y).o

##########################

I see in the Makefile that you do a lot of tricks to support various
kernel versions. But they all end up in a few defines for the C compiler,
which you just needs to supply in the variable LOOP_AES_CFLAGS.
[And some file massaging whaich is done before starting the build]

What I would recommend you to do is to move all your backward compatibility
stuff and general rules to a file named 'makefile'.
Then provide individual Makefiles for each kernel version:
Makefile.2.4, Makefile.2.6
Then symlink Makfile to the right one and build the module.

This would clean up your Makefile and give you correct usage in 2.6

	Sam



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

* Re: Linux 2.6.6-rc3
  2004-04-28 17:24     ` Jari Ruusu
  2004-04-28 19:08       ` Sam Ravnborg
@ 2004-04-28 19:18       ` Vincent C Jones
  1 sibling, 0 replies; 39+ messages in thread
From: Vincent C Jones @ 2004-04-28 19:18 UTC (permalink / raw)
  To: Jari Ruusu; +Cc: Sam Ravnborg, linux-kernel

On Wed, Apr 28, 2004 at 08:24:20PM +0300, Jari Ruusu wrote:
> Sam Ravnborg wrote:
> > On Wed, Apr 28, 2004 at 10:48:01AM -0400, Vincent C Jones wrote:
> > >
> > > loop-AES 2.0g has refused to compile since 2.6.6-rc1. No problems up
> > > through 2.6.5. The make script cd's to the kernel source root, then
> > > can't find any of the kernel include files, and it's all downhill from
> > > there...
> > 
> > This is an external module - right?
> > Could you mail me privately the Makefile used, or even better
> > the full package (or link to it).
> 
> Full package:
> http://loop-aes.sourceforge.net/loop-AES/loop-AES-v2.0g.tar.bz2
> 
> Patch to fix build problem:
> http://loop-aes.sourceforge.net/updates/loop-AES-v2.0g-20040412.diff.bz2
> 
> New version of loop-AES will be released shortly after final 2.6.6 kernel is
> released.
> 
> -- 
> Jari Ruusu  1024R/3A220F51 5B 4B F9 BB D3 3F 52 E9  DB 1D EB E3 24 0E A9 DD

Thanks much for the quick response. With the loop-AES makefile patched,
loop-AES and 2.6.6-rc3 are happy together.

One last question: Is there a reason there is no mention of the
compile problem or presence of a patch on the loop-AES sourceforge home
page (http://sourceforge.net/projects/loop-aes/)? Or is this common
knowledge that everyone is supposed to know about sourceforge projects?
At least with this thread, Google searches will no longer come up empty.

Now all I need is an APM implementation which suspends while on A/C
power and I can stop playing with kernel releases and concentrate
on work.

Thanks again!

Vince

-- 
Dr. Vincent C. Jones, PE              Expert advice and a helping hand
Computer Network Consultant           for those who want to manage and
Networking Unlimited, Inc.            control their networking destiny
14 Dogwood Lane, Tenafly, NJ 07670
http://www.networkingunlimited.com
VCJones@NetworkingUnlimited.com  +1 201 568-7810  Fax: +1 201 568-7269 

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

* 2.6.6-rc3: modular DVB tda1004x broken
  2004-04-28 11:56 ` Eyal Lebedinsky
@ 2004-05-01 20:13   ` Adrian Bunk
  2004-05-01 22:02     ` Johannes Stezenbach
                       ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Adrian Bunk @ 2004-05-01 20:13 UTC (permalink / raw)
  To: Eyal Lebedinsky, linux-dvb-maintainer; +Cc: Linus Torvalds, Kernel Mailing List

On Wed, Apr 28, 2004 at 09:56:08PM +1000, Eyal Lebedinsky wrote:
>...
> depmod says:
> 
> WARNING: 
> /lib/modules/2.6.6-rc3/kernel/drivers/media/dvb/frontends/tda1004x.ko needs 
> unknown symbol errno
>...

Thanks for this report.

It seems the DVB updates broke this.

Please _undo_ the patch below.

cu
Adrian

--- a/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
+++ b/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
@@ -188,7 +190,6 @@
 static struct fwinfo tda10046h_fwinfo[] = { {.file_size = 286720,.fw_offset = 0x3c4f9,.fw_size = 24479} };
 static int tda10046h_fwinfo_count = sizeof(tda10046h_fwinfo) / sizeof(struct fwinfo);
 
-static int errno;
 
 
 static int tda1004x_write_byte(struct dvb_i2c_bus *i2c, struct tda1004x_state *tda_state, int reg, int data)

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 20:13   ` 2.6.6-rc3: modular DVB tda1004x broken Adrian Bunk
@ 2004-05-01 22:02     ` Johannes Stezenbach
  2004-05-01 22:14     ` Andrew Morton
  2004-05-01 22:37     ` Linus Torvalds
  2 siblings, 0 replies; 39+ messages in thread
From: Johannes Stezenbach @ 2004-05-01 22:02 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Eyal Lebedinsky, linux-dvb-maintainer, Linus Torvalds,
	Kernel Mailing List

On Sat, May 01, 2004 at 10:13:42PM +0200, Adrian Bunk wrote:
> On Wed, Apr 28, 2004 at 09:56:08PM +1000, Eyal Lebedinsky wrote:
> >...
> > depmod says:
> > 
> > WARNING: 
> > /lib/modules/2.6.6-rc3/kernel/drivers/media/dvb/frontends/tda1004x.ko needs 
> > unknown symbol errno
> >...
> 
> Thanks for this report.
> 
> It seems the DVB updates broke this.
> 
> Please _undo_ the patch below.
...
> --- a/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
> +++ b/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
> @@ -188,7 +190,6 @@
>  static struct fwinfo tda10046h_fwinfo[] = { {.file_size = 286720,.fw_offset = 0x3c4f9,.fw_size = 24479} };
>  static int tda10046h_fwinfo_count = sizeof(tda10046h_fwinfo) / sizeof(struct fwinfo);
>  
> -static int errno;
>  
>  
>  static int tda1004x_write_byte(struct dvb_i2c_bus *i2c, struct tda1004x_state *tda_state, int reg, int data)

Indeed, errno is referenced by the __KERNEL_SYSCALLS__ cruft (still used for
firmware loading, request_firmware() depends on the patches that
Michael Hunold is working on for making DVB use the kernel I2C subsytem).
One more hint that this has to be done rsn...

I can't find the error report which motivated this patch so I cannot
come up with a different fix right now. Anyway, the patch must be
reverted.

Johannes

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 20:13   ` 2.6.6-rc3: modular DVB tda1004x broken Adrian Bunk
  2004-05-01 22:02     ` Johannes Stezenbach
@ 2004-05-01 22:14     ` Andrew Morton
  2004-05-01 22:37     ` Linus Torvalds
  2 siblings, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2004-05-01 22:14 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: eyal, linux-dvb-maintainer, torvalds, linux-kernel

Adrian Bunk <bunk@fs.tum.de> wrote:
>
> Please _undo_ the patch below.
> 
>  cu
>  Adrian
> 
>  --- a/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
>  +++ b/drivers/media/dvb/frontends/tda1004x.c	Tue Apr 27 18:37:15 2004
>  @@ -188,7 +190,6 @@
>   static struct fwinfo tda10046h_fwinfo[] = { {.file_size = 286720,.fw_offset = 0x3c4f9,.fw_size = 24479} };
>   static int tda10046h_fwinfo_count = sizeof(tda10046h_fwinfo) / sizeof(struct fwinfo);
>   
>  -static int errno;

Would be better to export sys_open() and sys_lseek() to GPL modules rather
than persisting with this cruft.

This driver was going to be converted to use the firmware loader API?

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 20:13   ` 2.6.6-rc3: modular DVB tda1004x broken Adrian Bunk
  2004-05-01 22:02     ` Johannes Stezenbach
  2004-05-01 22:14     ` Andrew Morton
@ 2004-05-01 22:37     ` Linus Torvalds
  2004-05-01 23:10       ` Andrew Morton
  2 siblings, 1 reply; 39+ messages in thread
From: Linus Torvalds @ 2004-05-01 22:37 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Eyal Lebedinsky, linux-dvb-maintainer, Kernel Mailing List



On Sat, 1 May 2004, Adrian Bunk wrote:
> 
> It seems the DVB updates broke this.
> 
> Please _undo_ the patch below.

No, there's something wrong. Nobody should use a global "errno" variable, 
and we should fix the real bug (it's probably some buggy system call 
"interface" function that is being used).

Can somebody who sees this problem please try to figure out where the 
buggy user of "errno" is?

		Linus

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 22:37     ` Linus Torvalds
@ 2004-05-01 23:10       ` Andrew Morton
  2004-05-01 23:55         ` Linus Torvalds
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Morton @ 2004-05-01 23:10 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: bunk, eyal, linux-dvb-maintainer, linux-kernel

Linus Torvalds <torvalds@osdl.org> wrote:
>
> 
> 
> On Sat, 1 May 2004, Adrian Bunk wrote:
> > 
> > It seems the DVB updates broke this.
> > 
> > Please _undo_ the patch below.
> 
> No, there's something wrong. Nobody should use a global "errno" variable, 
> and we should fix the real bug (it's probably some buggy system call 
> "interface" function that is being used).
> 
> Can somebody who sees this problem please try to figure out where the 
> buggy user of "errno" is?

It's using open() and lseek(), via KERNEL_SYSCALLS.

Maybe we should change __syscall_return() to return the -ve errno rather
than -1?


diff -puN include/asm-i386/unistd.h~a include/asm-i386/unistd.h
--- 25/include/asm-i386/unistd.h~a	2004-05-01 16:09:35.115389384 -0700
+++ 25-akpm/include/asm-i386/unistd.h	2004-05-01 16:09:49.513200584 -0700
@@ -295,10 +295,6 @@
 
 #define __syscall_return(type, res) \
 do { \
-	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
-		errno = -(res); \
-		res = -1; \
-	} \
 	return (type) (res); \
 } while (0)
 

_


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
       [not found]       ` <1RbW9-8sE-17@gated-at.bofh.it>
@ 2004-05-01 23:26         ` Andi Kleen
  2004-05-01 23:34           ` Andrew Morton
  0 siblings, 1 reply; 39+ messages in thread
From: Andi Kleen @ 2004-05-01 23:26 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

Andrew Morton <akpm@osdl.org> writes:
>
> Maybe we should change __syscall_return() to return the -ve errno rather
> than -1?
>
>
> diff -puN include/asm-i386/unistd.h~a include/asm-i386/unistd.h
> --- 25/include/asm-i386/unistd.h~a	2004-05-01 16:09:35.115389384 -0700
> +++ 25-akpm/include/asm-i386/unistd.h	2004-05-01 16:09:49.513200584 -0700
> @@ -295,10 +295,6 @@
>  
>  #define __syscall_return(type, res) \
>  do { \
> -	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
> -		errno = -(res); \
> -		res = -1; \
> -	} \
>  	return (type) (res); \
>  } while (0)

Don't do that please. That will break all the user space
programs who use asm/unistd.h to define own system calls
(it is quite common).

Make it conditional on __KERNEL__

-Andi
  


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 23:26         ` 2.6.6-rc3: modular DVB tda1004x broken Andi Kleen
@ 2004-05-01 23:34           ` Andrew Morton
  0 siblings, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2004-05-01 23:34 UTC (permalink / raw)
  To: Andi Kleen; +Cc: linux-kernel

Andi Kleen <ak@muc.de> wrote:
>
>  > diff -puN include/asm-i386/unistd.h~a include/asm-i386/unistd.h
>  > --- 25/include/asm-i386/unistd.h~a	2004-05-01 16:09:35.115389384 -0700
>  > +++ 25-akpm/include/asm-i386/unistd.h	2004-05-01 16:09:49.513200584 -0700
>  > @@ -295,10 +295,6 @@
>  >  
>  >  #define __syscall_return(type, res) \
>  >  do { \
>  > -	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
>  > -		errno = -(res); \
>  > -		res = -1; \
>  > -	} \
>  >  	return (type) (res); \
>  >  } while (0)
> 
>  Don't do that please. That will break all the user space
>  programs who use asm/unistd.h to define own system calls
>  (it is quite common).
> 
>  Make it conditional on __KERNEL__

err, that was just a "technology demonstration".  Obviously there's a lot
more involved.  Such as fixing up all the other architectures and then
killing off all the `errno' users.


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 23:10       ` Andrew Morton
@ 2004-05-01 23:55         ` Linus Torvalds
  2004-05-02  0:00           ` Chris Wedgwood
  2004-05-02  0:51           ` Andrew Morton
  0 siblings, 2 replies; 39+ messages in thread
From: Linus Torvalds @ 2004-05-01 23:55 UTC (permalink / raw)
  To: Andrew Morton; +Cc: bunk, eyal, linux-dvb-maintainer, linux-kernel



On Sat, 1 May 2004, Andrew Morton wrote:
> 
> Maybe we should change __syscall_return() to return the -ve errno rather
> than -1?

Yes. Except we should probably only do this for __KERNEL_SYSCALLS__, since
it's possible that somebody is still using this in user space (it
pre-glibc people).

So maybe a

	#undef __syscall_return
	#define __syscall_return(type, res)	return (type)(res);

inside the __KERNEL_SYSCALLS__ area?

		Linus

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 23:55         ` Linus Torvalds
@ 2004-05-02  0:00           ` Chris Wedgwood
  2004-05-02  0:03             ` Andrew Morton
  2004-05-02  0:28             ` Linus Torvalds
  2004-05-02  0:51           ` Andrew Morton
  1 sibling, 2 replies; 39+ messages in thread
From: Chris Wedgwood @ 2004-05-02  0:00 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, bunk, eyal, linux-dvb-maintainer, linux-kernel

On Sat, May 01, 2004 at 04:55:30PM -0700, Linus Torvalds wrote:

> Yes. Except we should probably only do this for __KERNEL_SYSCALLS__,
> since it's possible that somebody is still using this in user space
> (it pre-glibc people).

I'm confused.

I thought it has been decreed using kernel headers in userspace was a
bad idea (DONT DO THAT) so in theory we can just ignore this issue?


  --cw

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-02  0:00           ` Chris Wedgwood
@ 2004-05-02  0:03             ` Andrew Morton
  2004-05-02  0:28             ` Linus Torvalds
  1 sibling, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2004-05-02  0:03 UTC (permalink / raw)
  To: Chris Wedgwood; +Cc: torvalds, bunk, eyal, linux-dvb-maintainer, linux-kernel

Chris Wedgwood <cw@f00f.org> wrote:
>
> On Sat, May 01, 2004 at 04:55:30PM -0700, Linus Torvalds wrote:
> 
> > Yes. Except we should probably only do this for __KERNEL_SYSCALLS__,
> > since it's possible that somebody is still using this in user space
> > (it pre-glibc people).
> 
> I'm confused.
> 
> I thought it has been decreed using kernel headers in userspace was a
> bad idea (DONT DO THAT) so in theory we can just ignore this issue?
> 

I prefer not to break stuff which people are currently using, particularly
when avoiding the breakage is a simple thing.

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-02  0:00           ` Chris Wedgwood
  2004-05-02  0:03             ` Andrew Morton
@ 2004-05-02  0:28             ` Linus Torvalds
  1 sibling, 0 replies; 39+ messages in thread
From: Linus Torvalds @ 2004-05-02  0:28 UTC (permalink / raw)
  To: Chris Wedgwood
  Cc: Andrew Morton, bunk, eyal, linux-dvb-maintainer, linux-kernel



On Sat, 1 May 2004, Chris Wedgwood wrote:
> 
> I'm confused.

We all are, don't worry.

> I thought it has been decreed using kernel headers in userspace was a
> bad idea (DONT DO THAT) so in theory we can just ignore this issue?

Absolutely. But "in theory" is a thing we may want to strive for, but not 
at the expense of "in practice".

Sadly, we used to encourage (yeah, yeah, I should 'fess up: it was me, I'm 
guilty, I was stupid) user-space code to include kernel headers. 
Admittedly, that was about ten years ago, and we've tried to fix it ever 
since, but the thing is, I think backwards compatibility in the end is 
more important than "in theory". And silently breaking things in subtle 
ways would be bad.

What we _could_ do is to move _all_ the "_syscallX()" stuff into the
__KERNEL__ define, which would at least break any potential pre-glibc
users in a very visible way. What I _don't_ want to do is to have somebody
by mistake compile against updated kernel headers, and it still compiles,
but just doesn't work at run-time the way it's supposed to. THAT is
confusing and bad.

In short: either we should make non-kernel users break in _really_ obvious
(and hopefully easy-to-fix) ways, or we should keep things compatible. 
None of the "oh, the return value changed subtly" thing, please ;)

		Linus

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-01 23:55         ` Linus Torvalds
  2004-05-02  0:00           ` Chris Wedgwood
@ 2004-05-02  0:51           ` Andrew Morton
  2004-05-02  1:16             ` Paul Mackerras
  2004-05-03 18:06             ` David Mosberger
  1 sibling, 2 replies; 39+ messages in thread
From: Andrew Morton @ 2004-05-02  0:51 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: bunk, eyal, linux-dvb-maintainer, linux-kernel

Linus Torvalds <torvalds@osdl.org> wrote:
>
> So maybe a
> 
>  	#undef __syscall_return
>  	#define __syscall_return(type, res)	return (type)(res);
> 
>  inside the __KERNEL_SYSCALLS__ area?

It wasn't quite that simple - lots of architectures have been inventive.

For 2.6.6 we should just stick that errno decl back in there..


 25-akpm/include/asm-alpha/unistd.h     |    4 ++
 25-akpm/include/asm-arm/unistd.h       |    4 ++
 25-akpm/include/asm-arm26/unistd.h     |    4 ++
 25-akpm/include/asm-h8300/unistd.h     |    4 ++
 25-akpm/include/asm-i386/unistd.h      |    4 ++
 25-akpm/include/asm-m68k/unistd.h      |    4 ++
 25-akpm/include/asm-m68knommu/unistd.h |   45 ++++++---------------------
 25-akpm/include/asm-mips/unistd.h      |   55 +++++++++++----------------------
 25-akpm/include/asm-parisc/unistd.h    |   17 +++++++---
 25-akpm/include/asm-ppc/unistd.h       |   17 +++++++---
 25-akpm/include/asm-ppc64/unistd.h     |   17 +++++++---
 25-akpm/include/asm-s390/unistd.h      |    4 ++
 25-akpm/include/asm-sh/unistd.h        |    4 ++
 25-akpm/include/asm-sparc/unistd.h     |   40 +++++++++---------------
 25-akpm/include/asm-sparc64/unistd.h   |   40 +++++++++---------------
 25-akpm/include/asm-v850/unistd.h      |    5 ++-
 25-akpm/include/asm-x86_64/unistd.h    |    4 ++
 include/asm-cris/unistd.h              |    0 
 include/asm-ia64/unistd.h              |    0 
 include/asm-um/unistd.h                |    0 
 20 files changed, 138 insertions(+), 134 deletions(-)

diff -puN include/asm-i386/unistd.h~kernel-syscalls-retval-fix include/asm-i386/unistd.h
--- 25/include/asm-i386/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:04:04.998291624 -0700
+++ 25-akpm/include/asm-i386/unistd.h	2004-05-01 17:38:51.646072944 -0700
@@ -293,6 +293,9 @@
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
@@ -301,6 +304,7 @@ do { \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 /* XXX - _foo needs to be __foo, while __NR_bar could be _NR_bar. */
 #define _syscall0(type,name) \
diff -puN include/asm-alpha/unistd.h~kernel-syscalls-retval-fix include/asm-alpha/unistd.h
--- 25/include/asm-alpha/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:00.857678320 -0700
+++ 25-akpm/include/asm-alpha/unistd.h	2004-05-01 17:09:31.219698432 -0700
@@ -367,8 +367,12 @@
 
 #if defined(__GNUC__)
 
+#ifdef __KERNEL__
+#define _syscall_return(type)	return ((type) _sc_ret)
+#else
 #define _syscall_return(type)						\
 	return (_sc_err ? errno = _sc_ret, _sc_ret = -1L : 0), (type) _sc_ret
+#endif
 
 #define _syscall_clobbers						\
 	"$1", "$2", "$3", "$4", "$5", "$6", "$7", "$8",			\
diff -puN include/asm-arm26/unistd.h~kernel-syscalls-retval-fix include/asm-arm26/unistd.h
--- 25/include/asm-arm26/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:00.888673608 -0700
+++ 25-akpm/include/asm-arm26/unistd.h	2004-05-01 17:10:05.165537880 -0700
@@ -277,6 +277,9 @@
 #define __syscall(name) "swi\t" __sys1(__NR_##name) "\n\t"
 #endif
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res)					\
 do {									\
 	if ((unsigned long)(res) >= (unsigned long)(-125)) {		\
@@ -285,6 +288,7 @@ do {									\
 	}								\
 	return (type) (res);						\
 } while (0)
+#endif
 
 #define _syscall0(type,name)						\
 type name(void) {							\
diff -puN include/asm-arm/unistd.h~kernel-syscalls-retval-fix include/asm-arm/unistd.h
--- 25/include/asm-arm/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:00.917669200 -0700
+++ 25-akpm/include/asm-arm/unistd.h	2004-05-01 17:10:16.747777112 -0700
@@ -324,6 +324,9 @@
 #endif
 #endif
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res)					\
 do {									\
 	if ((unsigned long)(res) >= (unsigned long)(-125)) {		\
@@ -332,6 +335,7 @@ do {									\
 	}								\
 	return (type) (res);						\
 } while (0)
+#endif
 
 #define _syscall0(type,name)						\
 type name(void) {							\
diff -puN include/asm-cris/unistd.h~kernel-syscalls-retval-fix include/asm-cris/unistd.h
diff -puN include/asm-h8300/unistd.h~kernel-syscalls-retval-fix include/asm-h8300/unistd.h
--- 25/include/asm-h8300/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:00.971660992 -0700
+++ 25-akpm/include/asm-h8300/unistd.h	2004-05-01 17:10:55.685857624 -0700
@@ -276,6 +276,9 @@
 /* user-visible error numbers are in the range -1 - -122: see
    <asm-m68k/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
@@ -287,6 +290,7 @@ do { \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 #define _syscall0(type, name)							\
 type name(void)									\
diff -puN include/asm-ia64/unistd.h~kernel-syscalls-retval-fix include/asm-ia64/unistd.h
diff -puN include/asm-m68knommu/unistd.h~kernel-syscalls-retval-fix include/asm-m68knommu/unistd.h
--- 25/include/asm-m68knommu/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.069646096 -0700
+++ 25-akpm/include/asm-m68knommu/unistd.h	2004-05-01 17:14:51.502008120 -0700
@@ -226,17 +226,18 @@
 /* user-visible error numbers are in the range -1 - -122: see
    <asm-m68k/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
-	/* avoid using res which is declared to be in register d0; \
-	   errno might expand to a function call and clobber it.  */ \
-		int __err = -(res); \
-		errno = __err; \
+		errno = -(res); \
 		res = -1; \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 #define _syscall0(type, name)							\
 type name(void)									\
@@ -248,11 +249,7 @@ type name(void)									\
 			: "=g" (__res)						\
 			: "i" (__NR_##name)					\
 			: "cc", "%d0");						\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 #define _syscall1(type, name, atype, a)						\
@@ -267,11 +264,7 @@ type name(atype a)								\
 			: "i" (__NR_##name),					\
 			  "g" ((long)a)						\
 			: "cc", "%d0", "%d1");					\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 #define _syscall2(type, name, atype, a, btype, b)				\
@@ -288,11 +281,7 @@ type name(atype a, btype b)							\
 			  "a" ((long)a),					\
 			  "g" ((long)b)						\
 			: "cc", "%d0", "%d1", "%d2");				\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 #define _syscall3(type, name, atype, a, btype, b, ctype, c)			\
@@ -311,11 +300,7 @@ type name(atype a, btype b, ctype c)				
 			  "a" ((long)b),					\
 			  "g" ((long)c)						\
 			: "cc", "%d0", "%d1", "%d2", "%d3");			\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 #define _syscall4(type, name, atype, a, btype, b, ctype, c, dtype, d)		\
@@ -337,11 +322,7 @@ type name(atype a, btype b, ctype c, dty
 			  "g" ((long)d)						\
 			: "cc", "%d0", "%d1", "%d2", "%d3",			\
 			  "%d4");						\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 #define _syscall5(type, name, atype, a, btype, b, ctype, c, dtype, d, etype, e)	\
@@ -365,11 +346,7 @@ type name(atype a, btype b, ctype c, dty
 			  "g" ((long)e)						\
 			: "cc", "%d0", "%d1", "%d2", "%d3",			\
 			  "%d4", "%d5");					\
-  if ((unsigned long)(__res) >= (unsigned long)(-125)) {				\
-    errno = -__res;								\
-    __res = -1;									\
-  }										\
-  return (type)__res;								\
+  __syscall_return(type, __res);						\
 }
 
 
diff -puN include/asm-m68k/unistd.h~kernel-syscalls-retval-fix include/asm-m68k/unistd.h
--- 25/include/asm-m68k/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.097641840 -0700
+++ 25-akpm/include/asm-m68k/unistd.h	2004-05-01 17:15:10.603104312 -0700
@@ -244,6 +244,9 @@
 /* user-visible error numbers are in the range -1 - -124: see
    <asm-m68k/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
@@ -255,6 +258,7 @@ do { \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 #define _syscall0(type,name) \
 type name(void) \
diff -puN include/asm-mips/unistd.h~kernel-syscalls-retval-fix include/asm-mips/unistd.h
--- 25/include/asm-mips/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.132636520 -0700
+++ 25-akpm/include/asm-mips/unistd.h	2004-05-01 17:18:02.541965632 -0700
@@ -820,6 +820,16 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
+do { \
+	if (__a3 == 0) \
+		return (type)(res); \
+	errno = res; \
+	return -1; \
+} while (0)
+
 /* XXX - _foo needs to be __foo, while __NR_bar could be _NR_bar. */
 #define _syscall0(type,name) \
 type name(void) \
@@ -837,10 +847,7 @@ type name(void) \
 	: "i" (__NR_##name) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 /*
@@ -864,10 +871,7 @@ type name(atype a) \
 	: "r" (__a0), "i" (__NR_##name) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #define _syscall2(type,name,atype,a,btype,b) \
@@ -888,10 +892,7 @@ type name(atype a, btype b) \
 	: "r" (__a0), "r" (__a1), "i" (__NR_##name) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #define _syscall3(type,name,atype,a,btype,b,ctype,c) \
@@ -913,10 +914,7 @@ type name(atype a, btype b, ctype c) \
 	: "r" (__a0), "r" (__a1), "r" (__a2), "i" (__NR_##name) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #define _syscall4(type,name,atype,a,btype,b,ctype,c,dtype,d) \
@@ -938,10 +936,7 @@ type name(atype a, btype b, ctype c, dty
 	: "r" (__a0), "r" (__a1), "r" (__a2), "i" (__NR_##name) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #if (_MIPS_SIM == _MIPS_SIM_ABI32)
@@ -974,10 +969,7 @@ type name(atype a, btype b, ctype c, dty
 	  "m" ((unsigned long)e) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #define _syscall6(type,name,atype,a,btype,b,ctype,c,dtype,d,etype,e,ftype,f) \
@@ -1006,10 +998,7 @@ type name(atype a, btype b, ctype c, dty
 	  "m" ((unsigned long)e), "m" ((unsigned long)f) \
 	: "$2", "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #endif /* (_MIPS_SIM == _MIPS_SIM_ABI32) */
@@ -1036,10 +1025,7 @@ type name (atype a,btype b,ctype c,dtype
 	: "r" (__a0), "r" (__a1), "r" (__a2), "i" (__NR_##name) \
 	: "$2","$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #define _syscall6(type,name,atype,a,btype,b,ctype,c,dtype,d,etype,e,ftype,f) \
@@ -1064,10 +1050,7 @@ type name (atype a,btype b,ctype c,dtype
 	  "i" (__NR_##name) \
 	: "$2","$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24"); \
 	\
-	if (__a3 == 0) \
-		return (type) __v0; \
-	errno = __v0; \
-	return -1; \
+	__syscall_return(type, __v0) \
 }
 
 #endif /* (_MIPS_SIM == _MIPS_SIM_NABI32) || (_MIPS_SIM == _MIPS_SIM_ABI64) */
diff -puN include/asm-parisc/unistd.h~kernel-syscalls-retval-fix include/asm-parisc/unistd.h
--- 25/include/asm-parisc/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.161632112 -0700
+++ 25-akpm/include/asm-parisc/unistd.h	2004-05-01 17:20:18.759257456 -0700
@@ -739,6 +739,17 @@
 
 #define SYS_ify(syscall_name)   __NR_##syscall_name
 
+#ifdef __KERNEL__
+#define __syscall_return __sys_res
+#else
+#define __syscall_return					\
+        if (__sys_res >= (unsigned long)-4095) {		\
+		errno = -__sys_res;				\
+                __sys_res = (unsigned long)-1;			\
+        }							\
+        __sys_res;						\
+#endif
+
 /* The system call number MUST ALWAYS be loaded in the delay slot of
    the ble instruction, or restarting system calls WILL NOT WORK.  See
    arch/parisc/kernel/signal.c - dhd, 2000-07-26 */
@@ -755,11 +766,7 @@
 			  );                                    \
                 __sys_res = __res;                              \
         }                                                       \
-        if (__sys_res >= (unsigned long)-4095) {                \
-		errno = -__sys_res;				\
-                __sys_res = (unsigned long)-1;                 \
-        }                                                       \
-        __sys_res;                                              \
+	__syscall_return					\
 })
 
 #define K_LOAD_ARGS_0()
diff -puN include/asm-ppc64/unistd.h~kernel-syscalls-retval-fix include/asm-ppc64/unistd.h
--- 25/include/asm-ppc64/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.187628160 -0700
+++ 25-akpm/include/asm-ppc64/unistd.h	2004-05-01 17:21:43.897314504 -0700
@@ -286,6 +286,17 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef __KERNEL__
+#define __syscall_return	/* */
+#else
+#define __syscall_return						\
+	if (__sc_err & 0x10000000)					\
+	{								\
+		errno = __sc_ret;					\
+		__sc_ret = -1;						\
+	}								\
+#endif
+
 /* On powerpc a system call basically clobbers the same registers like a
  * function call, with the exception of LR (which is needed for the
  * "sc; bnslr" sequence) and CR (where only CR0.SO is clobbered to signal
@@ -317,11 +328,7 @@
 		__sc_ret = __sc_3;					\
 		__sc_err = __sc_0;					\
 	}								\
-	if (__sc_err & 0x10000000)					\
-	{								\
-		errno = __sc_ret;					\
-		__sc_ret = -1;						\
-	}								\
+	__syscall_return						\
 	return (type) __sc_ret
 
 #define __sc_loadargs_0(name, dummy...)					\
diff -puN include/asm-ppc/unistd.h~kernel-syscalls-retval-fix include/asm-ppc/unistd.h
--- 25/include/asm-ppc/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.222622840 -0700
+++ 25-akpm/include/asm-ppc/unistd.h	2004-05-01 17:22:19.506901024 -0700
@@ -277,6 +277,17 @@
 
 #define __NR(n)	#n
 
+#ifdef __KERNEL__
+#define __syscall_return	/* */
+#else
+#define __syscall_return						\
+	if (__sc_err & 0x10000000)					\
+	{								\
+		errno = __sc_ret;					\
+		__sc_ret = -1;						\
+	}								\
+#endif
+
 /* On powerpc a system call basically clobbers the same registers like a
  * function call, with the exception of LR (which is needed for the
  * "sc; bnslr" sequence) and CR (where only CR0.SO is clobbered to signal
@@ -307,11 +318,7 @@
 		__sc_ret = __sc_3;					\
 		__sc_err = __sc_0;					\
 	}								\
-	if (__sc_err & 0x10000000)					\
-	{								\
-		errno = __sc_ret;					\
-		__sc_ret = -1;						\
-	}								\
+	__syscall_return						\
 	return (type) __sc_ret
 
 #define __sc_loadargs_0(name, dummy...)					\
diff -puN include/asm-s390/unistd.h~kernel-syscalls-retval-fix include/asm-s390/unistd.h
--- 25/include/asm-s390/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.253618128 -0700
+++ 25-akpm/include/asm-s390/unistd.h	2004-05-01 17:22:38.607997216 -0700
@@ -360,6 +360,9 @@
 
 /* user-visible error numbers are in the range -1 - -122: see <asm-s390/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res)			     \
 do {							     \
 	if ((unsigned long)(res) >= (unsigned long)(-125)) { \
@@ -368,6 +371,7 @@ do {							     \
 	}						     \
 	return (type) (res);				     \
 } while (0)
+#endif
 
 #define _svc_clobber "1", "cc", "memory"
 
diff -puN include/asm-sh/unistd.h~kernel-syscalls-retval-fix include/asm-sh/unistd.h
--- 25/include/asm-sh/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.282613720 -0700
+++ 25-akpm/include/asm-sh/unistd.h	2004-05-01 17:22:57.037195552 -0700
@@ -286,6 +286,9 @@
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-sh/errno.h> */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-124)) { \
@@ -297,6 +300,7 @@ do { \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 /* XXX - _foo needs to be __foo, while __NR_bar could be _NR_bar. */
 #define _syscall0(type,name) \
diff -puN include/asm-sparc64/unistd.h~kernel-syscalls-retval-fix include/asm-sparc64/unistd.h
--- 25/include/asm-sparc64/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.324607336 -0700
+++ 25-akpm/include/asm-sparc64/unistd.h	2004-05-01 17:25:32.979488720 -0700
@@ -303,6 +303,16 @@
  *          find a free slot in the 0-282 range.
  */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
+#define __syscall_return(type, res)		\
+	if (res >= 0)				\
+		return (type)res;		\
+	errno = -res;				\
+	return -1;
+#endif
+
 #define _syscall0(type,name) \
 type name(void) \
 { \
@@ -314,10 +324,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res)\
 		      : "r" (__g1) \
 		      : "o0", "cc"); \
-if (__res >= 0) \
-    return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall1(type,name,type1,arg1) \
@@ -332,10 +339,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__g1) \
 		      : "cc"); \
-if (__res >= 0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall2(type,name,type1,arg1,type2,arg2) \
@@ -351,10 +355,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__g1) \
 		      : "cc"); \
-if (__res >= 0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall3(type,name,type1,arg1,type2,arg2,type3,arg3) \
@@ -371,10 +372,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__g1) \
 		      : "cc"); \
-if (__res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall4(type,name,type1,arg1,type2,arg2,type3,arg3,type4,arg4) \
@@ -392,10 +390,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__o3), "r" (__g1) \
 		      : "cc"); \
-if (__res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 } 
 
 #define _syscall5(type,name,type1,arg1,type2,arg2,type3,arg3,type4,arg4, \
@@ -415,10 +410,7 @@ __asm__ __volatile__ ("t 0x6d\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__o3), "r" (__o4), "r" (__g1) \
 		      : "cc"); \
-if (__res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 #ifdef __KERNEL_SYSCALLS__
 
diff -puN include/asm-sparc/unistd.h~kernel-syscalls-retval-fix include/asm-sparc/unistd.h
--- 25/include/asm-sparc/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.360601864 -0700
+++ 25-akpm/include/asm-sparc/unistd.h	2004-05-01 17:26:55.061010432 -0700
@@ -302,6 +302,16 @@
  *          find a free slot in the 0-282 range.
  */
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
+#define __syscall_return(type, res)		\
+	if (res < -255 || res >= 0)		\
+		return (type)res;		\
+	errno = -res;				\
+	return -1;
+#endif
+
 #define _syscall0(type,name) \
 type name(void) \
 { \
@@ -315,10 +325,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res)\
 		      : "r" (__g1) \
 		      : "o0", "cc"); \
-if (__res < -255 || __res >= 0) \
-    return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall1(type,name,type1,arg1) \
@@ -335,10 +342,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__g1) \
 		      : "cc"); \
-if (__res < -255 || __res >= 0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall2(type,name,type1,arg1,type2,arg2) \
@@ -356,10 +360,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__g1) \
 		      : "cc"); \
-if (__res < -255 || __res >= 0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall3(type,name,type1,arg1,type2,arg2,type3,arg3) \
@@ -378,10 +379,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__g1) \
 		      : "cc"); \
-if (__res < -255 || __res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 
 #define _syscall4(type,name,type1,arg1,type2,arg2,type3,arg3,type4,arg4) \
@@ -401,10 +399,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__o3), "r" (__g1) \
 		      : "cc"); \
-if (__res < -255 || __res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 } 
 
 #define _syscall5(type,name,type1,arg1,type2,arg2,type3,arg3,type4,arg4, \
@@ -426,10 +421,7 @@ __asm__ __volatile__ ("t 0x10\n\t" \
 		      : "=r" (__res), "=&r" (__o0) \
 		      : "1" (__o0), "r" (__o1), "r" (__o2), "r" (__o3), "r" (__o4), "r" (__g1) \
 		      : "cc"); \
-if (__res < -255 || __res>=0) \
-	return (type) __res; \
-errno = -__res; \
-return -1; \
+	__syscall_return(type, __res) \
 }
 #ifdef __KERNEL_SYSCALLS__
 
diff -puN include/asm-um/unistd.h~kernel-syscalls-retval-fix include/asm-um/unistd.h
diff -puN include/asm-v850/unistd.h~kernel-syscalls-retval-fix include/asm-v850/unistd.h
--- 25/include/asm-v850/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.415593504 -0700
+++ 25-akpm/include/asm-v850/unistd.h	2004-05-01 17:27:26.784187776 -0700
@@ -245,6 +245,9 @@
 #define __builtin_expect(x, expected_value) (x)
 #endif
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res)					      \
   do {									      \
 	  /* user-visible error numbers are in the range -1 - -124:	      \
@@ -255,7 +258,7 @@
 	  }								      \
 	  return (type) (res);						      \
   } while (0)
-
+#endif
 
 #define _syscall0(type, name)						      \
 type name (void)							      \
diff -puN include/asm-x86_64/unistd.h~kernel-syscalls-retval-fix include/asm-x86_64/unistd.h
--- 25/include/asm-x86_64/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.449588336 -0700
+++ 25-akpm/include/asm-x86_64/unistd.h	2004-05-01 17:27:40.526098688 -0700
@@ -560,6 +560,9 @@ __SYSCALL(__NR_mq_getsetattr, sys_mq_get
 
 #define __syscall_clobber "r11","rcx","memory" 
 
+#ifdef __KERNEL__
+#define __syscall_return(type, res) return ((type)(res))
+#else
 #define __syscall_return(type, res) \
 do { \
 	if ((unsigned long)(res) >= (unsigned long)(-127)) { \
@@ -568,6 +571,7 @@ do { \
 	} \
 	return (type) (res); \
 } while (0)
+#endif
 
 #ifndef __KERNEL_SYSCALLS__
 

_


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-02  0:51           ` Andrew Morton
@ 2004-05-02  1:16             ` Paul Mackerras
  2004-05-03 18:06             ` David Mosberger
  1 sibling, 0 replies; 39+ messages in thread
From: Paul Mackerras @ 2004-05-02  1:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linus Torvalds, bunk, eyal, linux-dvb-maintainer, linux-kernel

Andrew Morton writes:

> diff -puN include/asm-ppc64/unistd.h~kernel-syscalls-retval-fix include/asm-ppc64/unistd.h
> --- 25/include/asm-ppc64/unistd.h~kernel-syscalls-retval-fix	2004-05-01 17:06:01.187628160 -0700
> +++ 25-akpm/include/asm-ppc64/unistd.h	2004-05-01 17:21:43.897314504 -0700
> @@ -286,6 +286,17 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#ifdef __KERNEL__
> +#define __syscall_return	/* */

For ppc and ppc64 a syscall that returns an error puts the positive
error number in r3 and sets the SO bit in CR0.  So if we are expecting
the negative error number as the return value for syscalls in the
kernel we will need this instead:

#define __syscall_return		\
	if (__sc_err & 0x10000000)	\
		__sc_ret = - __sc_ret;

for the #ifdef __KERNEL__ case for both ppc and ppc64.

Thanks,
Paul.

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-02  0:51           ` Andrew Morton
  2004-05-02  1:16             ` Paul Mackerras
@ 2004-05-03 18:06             ` David Mosberger
  2004-05-03 18:20               ` Linus Torvalds
  2004-05-03 20:42               ` Linus Torvalds
  1 sibling, 2 replies; 39+ messages in thread
From: David Mosberger @ 2004-05-03 18:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linus Torvalds, bunk, eyal, linux-dvb-maintainer, linux-kernel

>>>>> On Sat, 1 May 2004 17:51:34 -0700, Andrew Morton <akpm@osdl.org> said:

  Andrew> It wasn't quite that simple - lots of architectures have
  Andrew> been inventive.

  Andrew> For 2.6.6 we should just stick that errno decl back in
  Andrew> there..

Why not just disallow user-level use of the _syscall() macros.
syscall() is a much more portable and easier to implement alternative
which has been around "forever" (even SunOS supported it, IIRC).  I
suppose there is a slight performance loss (function-call vs.  inline)
but we're talking about system calls here...

Also, the _syscall() macros have never been supported at the
user-level on ia64 linux and most packages already know to use
syscall().  Unfortunately, some "clever" maintainers use syscall()
only inside #ifdef __ia64, but that would be easy to fix if _syscall()
gets discouraged on all linux platforms.

	--david

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 18:06             ` David Mosberger
@ 2004-05-03 18:20               ` Linus Torvalds
  2004-05-03 18:30                 ` David Mosberger
  2004-05-03 20:42               ` Linus Torvalds
  1 sibling, 1 reply; 39+ messages in thread
From: Linus Torvalds @ 2004-05-03 18:20 UTC (permalink / raw)
  To: davidm; +Cc: Andrew Morton, bunk, eyal, linux-dvb-maintainer, linux-kernel



On Mon, 3 May 2004, David Mosberger wrote:
> 
> Why not just disallow user-level use of the _syscall() macros.

I'm thinking we should disallow the system-calls entirely. User mode
should be using their own macros, and kernel mode should just do the
function call directly.

How much work would that be? I suspect it would be less work than worrying 
about the existing strange interfaces.

		Linus

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 18:20               ` Linus Torvalds
@ 2004-05-03 18:30                 ` David Mosberger
  2004-05-03 18:37                   ` Linus Torvalds
  0 siblings, 1 reply; 39+ messages in thread
From: David Mosberger @ 2004-05-03 18:30 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: davidm, Andrew Morton, bunk, eyal, linux-dvb-maintainer,
	linux-kernel

>>>>> On Mon, 3 May 2004 11:20:35 -0700 (PDT), Linus Torvalds <torvalds@osdl.org> said:

  Linus> On Mon, 3 May 2004, David Mosberger wrote:

  >>  Why not just disallow user-level use of the _syscall() macros.

  Linus> I'm thinking we should disallow the system-calls
  Linus> entirely. User mode should be using their own macros, and
  Linus> kernel mode should just do the function call directly.

I totally agree.  (Note that syscall() is a libc-provided routine.
The kernel has nothing to do with it.)

  Linus> How much work would that be? I suspect it would be less work
  Linus> than worrying about the existing strange interfaces.

Yes.  On ia64, I held off on getting rid of kernel-internal syscalls
entirely because I was too lazy to do kernel_thread().  However, a
while ago, there was a bug that forced us to implement kernel_thread()
via a direct call.  Lo and behold, the resulting code was not only a
lot faster, but it turned out to be short and clean, too.  Very likely
the situation would be similar for the other architectures that have
been holding off on implementing the kernel syscalls via direct calls.

	--david

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 18:30                 ` David Mosberger
@ 2004-05-03 18:37                   ` Linus Torvalds
  0 siblings, 0 replies; 39+ messages in thread
From: Linus Torvalds @ 2004-05-03 18:37 UTC (permalink / raw)
  To: davidm; +Cc: Andrew Morton, bunk, eyal, linux-dvb-maintainer, linux-kernel



On Mon, 3 May 2004, David Mosberger wrote:
> 
> Yes.  On ia64, I held off on getting rid of kernel-internal syscalls
> entirely because I was too lazy to do kernel_thread().

Yeah, there's a few special calls that need more than just a simple 
C-level call. kernel_thread() and execve() ends up being the main ones, I 
think.

On alpha, you could never do the "system call from kernel space" thing in
the first place, so alpha has always just open-coded the things rather
than play games with the "legacy way". Not hugely difficult, at most it
requires one or two assembler wrappers for those special calls that want a
"struct pt_regs" thing passed into them.

		Linus

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 18:06             ` David Mosberger
  2004-05-03 18:20               ` Linus Torvalds
@ 2004-05-03 20:42               ` Linus Torvalds
  2004-05-03 20:56                 ` David Mosberger
  2004-05-03 21:02                 ` Andrew Morton
  1 sibling, 2 replies; 39+ messages in thread
From: Linus Torvalds @ 2004-05-03 20:42 UTC (permalink / raw)
  To: David Mosberger, Andrew Morton
  Cc: bunk, eyal, linux-dvb-maintainer, Kernel Mailing List



How about this patch? Tested on x86 ("make allyesconfig") and the default 
G5 ppc64 config, and likely to work at least on alpha too, since I took 
the silly definitions from there. Others should be trivial to fix up.

Rule: every architecture needs to implement its own kernel "execve()" 
function some way. Everything else is done by the architecture-independent 
<linux/unistd.h> translation layer.

The only change here is that this makes "open()" and friends depend on the
"sys_open()" and friends EXPORT's for modules. Right now it appears that
sys_open/sys_lseek/sys_read are all EXPORT_SYMBOL_GPL's. That sounds
pretty insane anyway (it's not like we can claim that "sys_open()" is some
_internal_ interface), so I'd be inclined to just change them all to
regular EXPORT_SYMBOL's.

(Yes, I realize that we want to _deprecate_ the use of open/read/lseek etc
from modules, but that's different from claiming that they are somehow
GPL-only things. If anything, we should deprecate them from our own 
_internal_ GPL usage _first_ rather than last).

Comments? To me, this is a pretty clear cleanup (and I left the old 
_syscallX() crud alone, even though we could remove it now entirely).

		Linus


------
===== include/asm-alpha/unistd.h 1.27 vs edited =====
--- 1.27/include/asm-alpha/unistd.h	Sat May  1 11:01:54 2004
+++ edited/include/asm-alpha/unistd.h	Mon May  3 13:01:53 2004
@@ -560,70 +560,8 @@
 
 #ifdef __KERNEL_SYSCALLS__
 
-#include <linux/compiler.h>
-#include <linux/types.h>
-#include <linux/string.h>
-#include <linux/signal.h>
-#include <linux/syscalls.h>
-#include <asm/ptrace.h>
-
-static inline long open(const char * name, int mode, int flags)
-{
-	return sys_open(name, mode, flags);
-}
-
-static inline long dup(int fd)
-{
-	return sys_dup(fd);
-}
-
-static inline long close(int fd)
-{
-	return sys_close(fd);
-}
-
-static inline off_t lseek(int fd, off_t off, int whence)
-{
-	return sys_lseek(fd, off, whence);
-}
-
-static inline void _exit(int value)
-{
-	sys_exit(value);
-}
-
-#define exit(x) _exit(x)
-
-static inline long write(int fd, const char * buf, size_t nr)
-{
-	return sys_write(fd, buf, nr);
-}
-
-static inline long read(int fd, char * buf, size_t nr)
-{
-	return sys_read(fd, buf, nr);
-}
-
+/* This needs a small assembly stub */
 extern long execve(char *, char **, char **);
-
-static inline long setsid(void)
-{
-	return sys_setsid();
-}
-
-static inline pid_t waitpid(int pid, int * wait_stat, int flags)
-{
-	return sys_wait4(pid, wait_stat, flags, NULL);
-}
-
-asmlinkage int sys_execve(char *ufilename, char **argv, char **envp,
-			unsigned long a3, unsigned long a4, unsigned long a5,
-			struct pt_regs regs);
-asmlinkage long sys_rt_sigaction(int sig,
-				const struct sigaction __user *act,
-				struct sigaction __user *oact,
-				size_t sigsetsize,
-				void *restorer);
 
 #endif /* __KERNEL_SYSCALLS__ */
 
===== include/asm-i386/unistd.h 1.35 vs edited =====
--- 1.35/include/asm-i386/unistd.h	Mon Apr 12 10:54:15 2004
+++ edited/include/asm-i386/unistd.h	Mon May  3 13:22:49 2004
@@ -382,49 +382,17 @@
 
 #ifdef __KERNEL_SYSCALLS__
 
-#include <linux/compiler.h>
-#include <linux/types.h>
-#include <linux/linkage.h>
-#include <asm/ptrace.h>
-
-/*
- * we need this inline - forking from kernel space will result
- * in NO COPY ON WRITE (!!!), until an execve is executed. This
- * is no problem, but for the stack. This is handled by not letting
- * main() use the stack at all after fork(). Thus, no function
- * calls - which means inline code for fork too, as otherwise we
- * would use the stack upon exit from 'fork()'.
- *
- * Actually only pause and fork are needed inline, so that there
- * won't be any messing with the stack from main(), but we define
- * some others too.
- */
-static inline _syscall0(pid_t,setsid)
-static inline _syscall3(int,write,int,fd,const char *,buf,off_t,count)
-static inline _syscall3(int,read,int,fd,char *,buf,off_t,count)
-static inline _syscall3(off_t,lseek,int,fd,off_t,offset,int,count)
-static inline _syscall1(int,dup,int,fd)
-static inline _syscall3(int,execve,const char *,file,char **,argv,char **,envp)
-static inline _syscall3(int,open,const char *,file,int,flag,int,mode)
-static inline _syscall1(int,close,int,fd)
-static inline _syscall3(pid_t,waitpid,pid_t,pid,int *,wait_stat,int,options)
-
-asmlinkage int sys_modify_ldt(int func, void __user *ptr, unsigned long bytecount);
-asmlinkage long sys_mmap2(unsigned long addr, unsigned long len,
-			unsigned long prot, unsigned long flags,
-			unsigned long fd, unsigned long pgoff);
-asmlinkage int sys_execve(struct pt_regs regs);
-asmlinkage int sys_clone(struct pt_regs regs);
-asmlinkage int sys_fork(struct pt_regs regs);
-asmlinkage int sys_vfork(struct pt_regs regs);
-asmlinkage int sys_pipe(unsigned long __user *fildes);
-asmlinkage int sys_ptrace(long request, long pid, long addr, long data);
-asmlinkage long sys_iopl(unsigned long unused);
-struct sigaction;
-asmlinkage long sys_rt_sigaction(int sig,
-				const struct sigaction __user *act,
-				struct sigaction __user *oact,
-				size_t sigsetsize);
+static inline int execve(const char *file, char **argv, char **envp)
+{
+	long __res;
+	__asm__ volatile ("int $0x80"
+		: "=a" (__res)
+		: "0" (__NR_execve),
+		  "b" (file),
+		  "c" (argv),
+		  "d" (envp));
+	return __res;
+}
 
 #endif
 
===== include/linux/unistd.h 1.1 vs edited =====
--- 1.1/include/linux/unistd.h	Tue Feb  5 09:39:39 2002
+++ edited/include/linux/unistd.h	Mon May  3 13:20:51 2004
@@ -8,4 +8,32 @@
  */
 #include <asm/unistd.h>
 
+#ifdef __KERNEL_SYSCALLS__
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/syscalls.h>
+
+static inline long open(const char * name, int mode, int flags)
+{
+	return sys_open((const char __user *) name, mode, flags);
+}
+
+static inline long close(int fd)
+{
+	return sys_close(fd);
+}
+
+static inline off_t lseek(int fd, off_t off, int whence)
+{
+	return sys_lseek(fd, off, whence);
+}
+
+static inline long read(int fd, char * buf, size_t nr)
+{
+	return sys_read(fd, (char __user *) buf, nr);
+}
+
+#endif /* __KERNEL_SYSCALLS__ */
+
 #endif /* _LINUX_UNISTD_H_ */

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 20:42               ` Linus Torvalds
@ 2004-05-03 20:56                 ` David Mosberger
  2004-05-03 21:07                   ` Arjan van de Ven
  2004-05-03 21:02                 ` Andrew Morton
  1 sibling, 1 reply; 39+ messages in thread
From: David Mosberger @ 2004-05-03 20:56 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Mosberger, Andrew Morton, bunk, eyal, linux-dvb-maintainer,
	Kernel Mailing List

>>>>> On Mon, 3 May 2004 13:42:54 -0700 (PDT), Linus Torvalds <torvalds@osdl.org> said:

  Linus> Rule: every architecture needs to implement its own kernel
  Linus> "execve()" function some way. Everything else is done by the
  Linus> architecture-independent <linux/unistd.h> translation layer.

Looks good to me.

  Linus> The only change here is that this makes "open()" and friends
  Linus> depend on the "sys_open()" and friends EXPORT's for
  Linus> modules. Right now it appears that
  Linus> sys_open/sys_lseek/sys_read are all EXPORT_SYMBOL_GPL's. That
  Linus> sounds pretty insane anyway (it's not like we can claim that
  Linus> "sys_open()" is some _internal_ interface), so I'd be
  Linus> inclined to just change them all to regular EXPORT_SYMBOL's.

Does the rule have to be that all sys_FOO() entry-points must be
exported via EXPORT_SYMBOL()?  Otherwise we have the strange issue
where a kernel-module may not be able to (easily) invoke a system call
which it could formerly invoke via _syscallN().  For example, the ATI
driver wants to call mlock()/munlock().  While this happens to be a
proprietary/binary-only driver, the same issue can arise with GPL'd
modules.

  Linus> Comments? To me, this is a pretty clear cleanup (and I left
  Linus> the old _syscallX() crud alone, even though we could remove
  Linus> it now entirely).

In my opinion, it would be good to remove _syscallX() altogether.

	--david

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 20:42               ` Linus Torvalds
  2004-05-03 20:56                 ` David Mosberger
@ 2004-05-03 21:02                 ` Andrew Morton
  2004-05-03 21:06                   ` Arjan van de Ven
  2004-05-03 21:16                   ` viro
  1 sibling, 2 replies; 39+ messages in thread
From: Andrew Morton @ 2004-05-03 21:02 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: davidm, bunk, eyal, linux-dvb-maintainer, linux-kernel

Linus Torvalds <torvalds@osdl.org> wrote:
>
> 
> 
> How about this patch? 

Seems sane.  For after 2.6.6 ;)

> +static inline long open(const char * name, int mode, int flags)
> +{
> +	return sys_open((const char __user *) name, mode, flags);
> +}

We may as well stick the get_fs()/set_fs() stuff in here as well - all
callers need to do it, after all.  After which it would best be uninlined.

We can then remove all those `errno's from all over the place, too.  I have
a patch for that.


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:02                 ` Andrew Morton
@ 2004-05-03 21:06                   ` Arjan van de Ven
  2004-05-03 21:16                   ` viro
  1 sibling, 0 replies; 39+ messages in thread
From: Arjan van de Ven @ 2004-05-03 21:06 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linus Torvalds, davidm, bunk, eyal, linux-dvb-maintainer,
	linux-kernel

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

On Mon, 2004-05-03 at 23:02, Andrew Morton wrote:
> Linus Torvalds <torvalds@osdl.org> wrote:
> >
> > 
> > 
> > How about this patch? 
> 
> Seems sane.  For after 2.6.6 ;)
> 
> > +static inline long open(const char * name, int mode, int flags)
> > +{
> > +	return sys_open((const char __user *) name, mode, flags);
> > +}
> 
> We may as well stick the get_fs()/set_fs() stuff in here as well - all
> callers need to do it, after all.  After which it would best be uninlined.


if you're going to modify callers, why not make them use open_filp()
instead of sys_open etc.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 20:56                 ` David Mosberger
@ 2004-05-03 21:07                   ` Arjan van de Ven
  2004-05-03 22:39                     ` David Mosberger
  0 siblings, 1 reply; 39+ messages in thread
From: Arjan van de Ven @ 2004-05-03 21:07 UTC (permalink / raw)
  To: davidm
  Cc: Linus Torvalds, Andrew Morton, bunk, eyal, linux-dvb-maintainer,
	Kernel Mailing List

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

>  For example, the ATI
> driver wants to call mlock()/munlock().  While this happens to be a
> proprietary/binary-only driver, the same issue can arise with GPL'd
> modules.

that is too ugly to live imo. Exporting sys_mlock() for a kernel module
soooo sounds wrong to me, regardless of binary only vs gpl.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:02                 ` Andrew Morton
  2004-05-03 21:06                   ` Arjan van de Ven
@ 2004-05-03 21:16                   ` viro
  2004-05-03 21:24                     ` Jörn Engel
  1 sibling, 1 reply; 39+ messages in thread
From: viro @ 2004-05-03 21:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Linus Torvalds, davidm, bunk, eyal, linux-dvb-maintainer,
	linux-kernel

On Mon, May 03, 2004 at 02:02:51PM -0700, Andrew Morton wrote:
> Linus Torvalds <torvalds@osdl.org> wrote:
> >
> > 
> > 
> > How about this patch? 
> 
> Seems sane.  For after 2.6.6 ;)
> 
> > +static inline long open(const char * name, int mode, int flags)
> > +{
> > +	return sys_open((const char __user *) name, mode, flags);
> > +}
> 
> We may as well stick the get_fs()/set_fs() stuff in here as well - all
> callers need to do it, after all.  After which it would best be uninlined.

I'd rather kill open() completely - we only have a handful of in-tree users
and there's no good reason to keep that crap, AFAICS.  I'm gathering the
list of in-tree callers of open()/lseek()/close() and so far a lot of them
look buggy.  More on that later...

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:16                   ` viro
@ 2004-05-03 21:24                     ` Jörn Engel
  2004-05-03 21:54                       ` viro
  0 siblings, 1 reply; 39+ messages in thread
From: Jörn Engel @ 2004-05-03 21:24 UTC (permalink / raw)
  To: viro
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Mon, 3 May 2004 22:16:07 +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> I'd rather kill open() completely - we only have a handful of in-tree users
> and there's no good reason to keep that crap, AFAICS.  I'm gathering the
> list of in-tree callers of open()/lseek()/close() and so far a lot of them
> look buggy.  More on that later...

Do you know how many of those exist purely for the purpose of passing
a struct file to one of the various read/write functions?

Jörn

-- 
The grand essentials of happiness are: something to do, something to
love, and something to hope for.
-- Allan K. Chalmers

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:24                     ` Jörn Engel
@ 2004-05-03 21:54                       ` viro
  2004-05-03 22:01                         ` viro
                                           ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: viro @ 2004-05-03 21:54 UTC (permalink / raw)
  To: Jörn Engel
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Mon, May 03, 2004 at 11:24:50PM +0200, Jörn Engel wrote:
> On Mon, 3 May 2004 22:16:07 +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > 
> > I'd rather kill open() completely - we only have a handful of in-tree users
> > and there's no good reason to keep that crap, AFAICS.  I'm gathering the
> > list of in-tree callers of open()/lseek()/close() and so far a lot of them
> > look buggy.  More on that later...
> 
> Do you know how many of those exist purely for the purpose of passing
> a struct file to one of the various read/write functions?

Let's see...  Leaving aside obvious userland code (arch/*/boot/*, scripts/*,
drivers/char/ip2/ helpers) and arch/um/* instances that are, AFAICS, in
userland code too and refer to libc open(2), we have
	a) drivers/media/dvb/frontends pile (AFAICS, loading firmware)
	b) systemcfg_init() in asm-ppc64/systemcfg.h (WTF is that about?)
	c) sound/isa/wavefront/wavefront_synth.c (loading firmware)
	d) sound/oss/wavfront.c (loading firmware)
	e) odd calls of sys_close() in binfmt_elf.c, eventpoll.c and socket.c
	f) potentially racy flush_unauthorized_files() in selinux code - uses
sys_close() in a strange way.

That's it.  And I suspect that we ought to switch the firmware loaders to
use of existing helper.

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:54                       ` viro
@ 2004-05-03 22:01                         ` viro
  2004-05-03 22:33                         ` Jörn Engel
  2004-05-04 12:13                         ` Stephen Smalley
  2 siblings, 0 replies; 39+ messages in thread
From: viro @ 2004-05-03 22:01 UTC (permalink / raw)
  To: Jörn Engel
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Mon, May 03, 2004 at 10:54:34PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> 	b) systemcfg_init() in asm-ppc64/systemcfg.h (WTF is that about?)

... outside of __KERNEL__, so looks like a userland code that got there
for fsck knows what reason.  Anyway, open() and close() in it are libc
ones.

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:54                       ` viro
  2004-05-03 22:01                         ` viro
@ 2004-05-03 22:33                         ` Jörn Engel
  2004-05-04  0:14                           ` viro
  2004-05-04 12:13                         ` Stephen Smalley
  2 siblings, 1 reply; 39+ messages in thread
From: Jörn Engel @ 2004-05-03 22:33 UTC (permalink / raw)
  To: viro
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Mon, 3 May 2004 22:54:34 +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> Let's see...  Leaving aside obvious userland code (arch/*/boot/*, scripts/*,
> drivers/char/ip2/ helpers) and arch/um/* instances that are, AFAICS, in
> userland code too and refer to libc open(2), we have
> 	a) drivers/media/dvb/frontends pile (AFAICS, loading firmware)
> 	b) systemcfg_init() in asm-ppc64/systemcfg.h (WTF is that about?)
> 	c) sound/isa/wavefront/wavefront_synth.c (loading firmware)
> 	d) sound/oss/wavfront.c (loading firmware)
> 	e) odd calls of sys_close() in binfmt_elf.c, eventpoll.c and socket.c
> 	f) potentially racy flush_unauthorized_files() in selinux code - uses
> sys_close() in a strange way.
> 
> That's it.  And I suspect that we ought to switch the firmware loaders to
> use of existing helper.

That cannot be all.  What about these two?  Not sure if my patch is
correct or it should rather s/O_RDONLY/FMODE_READ/.

Jörn

-- 
It does not matter how slowly you go, so long as you do not stop.
-- Confucius

 exec.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

--- linux-2.6.5cow/fs/exec.c~exec_thinko	2004-03-28 21:51:52.000000000 +0200
+++ linux-2.6.5cow/fs/exec.c	2004-05-02 11:47:46.000000000 +0200
@@ -135,7 +135,8 @@
 	if (error)
 		goto exit;
 
-	file = dentry_open(nd.dentry, nd.mnt, O_RDONLY);
+	/* dentry_open used FMODE_(READ|WRITE) instead of the O_* flags. */
+	file = dentry_open(nd.dentry, nd.mnt, 0);
 	error = PTR_ERR(file);
 	if (IS_ERR(file))
 		goto out;
@@ -489,7 +490,8 @@
 				err = -EACCES;
 			file = ERR_PTR(err);
 			if (!err) {
-				file = dentry_open(nd.dentry, nd.mnt, O_RDONLY);
+	/* dentry_open used FMODE_(READ|WRITE) instead of the O_* flags. */
+				file = dentry_open(nd.dentry, nd.mnt, 0);
 				if (!IS_ERR(file)) {
 					err = deny_write_access(file);
 					if (err) {

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:07                   ` Arjan van de Ven
@ 2004-05-03 22:39                     ` David Mosberger
  2004-05-04  7:55                       ` Arjan van de Ven
  0 siblings, 1 reply; 39+ messages in thread
From: David Mosberger @ 2004-05-03 22:39 UTC (permalink / raw)
  To: arjanv
  Cc: davidm, Linus Torvalds, Andrew Morton, bunk, eyal,
	linux-dvb-maintainer, Kernel Mailing List

>>>>> On Mon, 03 May 2004 23:07:04 +0200, Arjan van de Ven <arjanv@redhat.com> said:

  Arjan> Exporting sys_mlock() for a kernel module soooo sounds wrong
  Arjan> to me

On what grounds?  What alternative are you suggesting?

	--david

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 22:33                         ` Jörn Engel
@ 2004-05-04  0:14                           ` viro
  2004-05-04  9:23                             ` Jörn Engel
  0 siblings, 1 reply; 39+ messages in thread
From: viro @ 2004-05-04  0:14 UTC (permalink / raw)
  To: Jörn Engel
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Tue, May 04, 2004 at 12:33:17AM +0200, Jörn Engel wrote:
> That cannot be all.  What about these two?  Not sure if my patch is
> correct or it should rather s/O_RDONLY/FMODE_READ/.

WTF does that have to sys_open(9) or open(9)?

And no, dentry_open() does not take FMODE_... - open_namei() does,
but dentry_open() takes flags unchanged.  Just take a look at
fs/open.c::filp_open() - flags are passed unchanged.

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 22:39                     ` David Mosberger
@ 2004-05-04  7:55                       ` Arjan van de Ven
  2004-05-04 16:44                         ` David Mosberger
  0 siblings, 1 reply; 39+ messages in thread
From: Arjan van de Ven @ 2004-05-04  7:55 UTC (permalink / raw)
  To: davidm
  Cc: Linus Torvalds, Andrew Morton, bunk, eyal, linux-dvb-maintainer,
	Kernel Mailing List

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

On Mon, May 03, 2004 at 03:39:08PM -0700, David Mosberger wrote:
> >>>>> On Mon, 03 May 2004 23:07:04 +0200, Arjan van de Ven <arjanv@redhat.com> said:
> 
>   Arjan> Exporting sys_mlock() for a kernel module soooo sounds wrong
>   Arjan> to me
> 
> On what grounds?  What alternative are you suggesting?

if the module wants to pin userspace pages there are plenty of alternatives.
Heck I suspect what they want is to mmap a device like most V4L drivers do,
instead of doing it the windows way (let the app malloc and then pin)

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

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-04  0:14                           ` viro
@ 2004-05-04  9:23                             ` Jörn Engel
  0 siblings, 0 replies; 39+ messages in thread
From: Jörn Engel @ 2004-05-04  9:23 UTC (permalink / raw)
  To: viro
  Cc: Andrew Morton, Linus Torvalds, davidm, bunk, eyal,
	linux-dvb-maintainer, linux-kernel

On Tue, 4 May 2004 01:14:50 +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Tue, May 04, 2004 at 12:33:17AM +0200, Jörn Engel wrote:
> > That cannot be all.  What about these two?  Not sure if my patch is
> > correct or it should rather s/O_RDONLY/FMODE_READ/.
> 
> WTF does that have to sys_open(9) or open(9)?
> 
> And no, dentry_open() does not take FMODE_... - open_namei() does,
> but dentry_open() takes flags unchanged.  Just take a look at
> fs/open.c::filp_open() - flags are passed unchanged.

True, stupid me.

Jörn

-- 
It does not matter how slowly you go, so long as you do not stop.
-- Confucius

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-03 21:54                       ` viro
  2004-05-03 22:01                         ` viro
  2004-05-03 22:33                         ` Jörn Engel
@ 2004-05-04 12:13                         ` Stephen Smalley
  2 siblings, 0 replies; 39+ messages in thread
From: Stephen Smalley @ 2004-05-04 12:13 UTC (permalink / raw)
  To: Alexander Viro
  Cc: Jörn Engel, Andrew Morton, Linus Torvalds, davidm, bunk,
	eyal, linux-dvb-maintainer, lkml, James Morris

On Mon, 2004-05-03 at 17:54, viro@parcelfarce.linux.theplanet.co.uk
wrote:
> 	f) potentially racy flush_unauthorized_files() in selinux code - uses
> sys_close() in a strange way.

The SELinux flush_unauthorized_files code is based on flush_old_files in
fs/exec.c.  It is only executed upon a SID/context transition to flush
files that are not authorized for the new SID/context, and sharing of
the open file table across such transitions requires a share permission
that is only allowed where absolutely necessary, e.g. kernel->init.  Do
we need to change the code?

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency


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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-04  7:55                       ` Arjan van de Ven
@ 2004-05-04 16:44                         ` David Mosberger
  2004-05-04 16:47                           ` Arjan van de Ven
  0 siblings, 1 reply; 39+ messages in thread
From: David Mosberger @ 2004-05-04 16:44 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: davidm, Linus Torvalds, Andrew Morton, bunk, eyal,
	linux-dvb-maintainer, Kernel Mailing List

>>>>> On Tue, 4 May 2004 09:55:55 +0200, Arjan van de Ven <arjanv@redhat.com> said:

  Arjan> On Mon, May 03, 2004 at 03:39:08PM -0700, David Mosberger wrote:
  >> >>>>> On Mon, 03 May 2004 23:07:04 +0200, Arjan van de Ven <arjanv@redhat.com> said:

  Arjan> Exporting sys_mlock() for a kernel module soooo sounds wrong
  Arjan> to me

  >> On what grounds?  What alternative are you suggesting?

  Arjan> if the module wants to pin userspace pages there are plenty
  Arjan> of alternatives.  Heck I suspect what they want is to mmap a
  Arjan> device like most V4L drivers do, instead of doing it the
  Arjan> windows way (let the app malloc and then pin)

You may well be right (it's hard to tell what they're using mlock()
for since its called from the binary-only portion of the driver).
However, as long as x86 supports the _syscallN() macros, they won't
change.

	--david

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

* Re: 2.6.6-rc3: modular DVB tda1004x broken
  2004-05-04 16:44                         ` David Mosberger
@ 2004-05-04 16:47                           ` Arjan van de Ven
  0 siblings, 0 replies; 39+ messages in thread
From: Arjan van de Ven @ 2004-05-04 16:47 UTC (permalink / raw)
  To: davidm
  Cc: Linus Torvalds, Andrew Morton, bunk, eyal, linux-dvb-maintainer,
	Kernel Mailing List

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

On Tue, May 04, 2004 at 09:44:27AM -0700, David Mosberger wrote:
> 
> You may well be right (it's hard to tell what they're using mlock()
> for since its called from the binary-only portion of the driver).
> However, as long as x86 supports the _syscallN() macros, they won't
> change.

I got the impression that just changed ;)

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

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

end of thread, other threads:[~2004-05-04 16:50 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1PMQ9-5K6-3@gated-at.bofh.it>
2004-04-28 14:48 ` Linux 2.6.6-rc3 Vincent C Jones
2004-04-28 16:00   ` Sam Ravnborg
2004-04-28 17:24     ` Jari Ruusu
2004-04-28 19:08       ` Sam Ravnborg
2004-04-28 19:18       ` Vincent C Jones
     [not found] ` <1PVTx-4rY-25@gated-at.bofh.it>
     [not found]   ` <1R9hC-6rC-3@gated-at.bofh.it>
     [not found]     ` <1Rbjr-7Y5-17@gated-at.bofh.it>
     [not found]       ` <1RbW9-8sE-17@gated-at.bofh.it>
2004-05-01 23:26         ` 2.6.6-rc3: modular DVB tda1004x broken Andi Kleen
2004-05-01 23:34           ` Andrew Morton
2004-04-28  2:03 Linux 2.6.6-rc3 Linus Torvalds
2004-04-28 11:56 ` Eyal Lebedinsky
2004-05-01 20:13   ` 2.6.6-rc3: modular DVB tda1004x broken Adrian Bunk
2004-05-01 22:02     ` Johannes Stezenbach
2004-05-01 22:14     ` Andrew Morton
2004-05-01 22:37     ` Linus Torvalds
2004-05-01 23:10       ` Andrew Morton
2004-05-01 23:55         ` Linus Torvalds
2004-05-02  0:00           ` Chris Wedgwood
2004-05-02  0:03             ` Andrew Morton
2004-05-02  0:28             ` Linus Torvalds
2004-05-02  0:51           ` Andrew Morton
2004-05-02  1:16             ` Paul Mackerras
2004-05-03 18:06             ` David Mosberger
2004-05-03 18:20               ` Linus Torvalds
2004-05-03 18:30                 ` David Mosberger
2004-05-03 18:37                   ` Linus Torvalds
2004-05-03 20:42               ` Linus Torvalds
2004-05-03 20:56                 ` David Mosberger
2004-05-03 21:07                   ` Arjan van de Ven
2004-05-03 22:39                     ` David Mosberger
2004-05-04  7:55                       ` Arjan van de Ven
2004-05-04 16:44                         ` David Mosberger
2004-05-04 16:47                           ` Arjan van de Ven
2004-05-03 21:02                 ` Andrew Morton
2004-05-03 21:06                   ` Arjan van de Ven
2004-05-03 21:16                   ` viro
2004-05-03 21:24                     ` Jörn Engel
2004-05-03 21:54                       ` viro
2004-05-03 22:01                         ` viro
2004-05-03 22:33                         ` Jörn Engel
2004-05-04  0:14                           ` viro
2004-05-04  9:23                             ` Jörn Engel
2004-05-04 12:13                         ` Stephen Smalley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox