[patch] arch-specific cond_syscall usage issues

[patch] arch-specific cond_syscall usage issues

[Matt Mackall]

Experimenting with trying to use cond_syscall for a few arch-specific
syscalls, I discovered that it can't actually be used outside the file
in which sys_ni_syscall is declared because the assembler doesn't feel
obliged to output the symbol in that case:

weak.c:

#define cond_syscall(x) asm(".weak\t" #x "\n\t.set\t" #x ",sys_ni_syscall");
cond_syscall(sys_foo);

$ nm weak.o
         U sys_ni_syscall

One arch (PPC) is apparently trying to use cond_syscall this way
anyway, though it's probably never been actually tested as the above
test was done on a PPC.

After trying a bunch of tricks to get it to work nicely, I decided
there are basically two alternatives: make weak versions of
sys_ni_syscall wherever they're wanted or put the arch-specific
cond_syscalls in kernel/sys.c where sys_ni_syscall is defined.

The former approach is a bit crufty and doesn't actually do the right
thing in practice as you'll get multiple copies of sys_ni_syscall in
your final image.

The latter introduces some slight arch-pollution in sys.c, but as
arch-specific cond_syscalls aren't all that frequent, it should be
pretty minor. So here's a patch to move the current offender to sys.c:

 tiny-mpm/arch/ppc/kernel/syscalls.c |    2 --
 tiny-mpm/kernel/sys.c               |    3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff -puN arch/ppc/kernel/syscalls.c~ppc_cond_syscall arch/ppc/kernel/syscalls.c
--- tiny/arch/ppc/kernel/syscalls.c~ppc_cond_syscall	2004-01-09 21:15:02.000000000 -0600
+++ tiny-mpm/arch/ppc/kernel/syscalls.c	2004-01-09 21:15:08.000000000 -0600
@@ -271,5 +271,3 @@ long ppc_fadvise64_64(int fd, int advice
 {
 	return sys_fadvise64_64(fd, offset, len, advice);
 }
-
-cond_syscall(sys_pciconfig_iobase);
diff -puN kernel/sys.c~ppc_cond_syscall kernel/sys.c
--- tiny/kernel/sys.c~ppc_cond_syscall	2004-01-09 21:15:02.000000000 -0600
+++ tiny-mpm/kernel/sys.c	2004-01-09 21:15:02.000000000 -0600
@@ -252,6 +252,9 @@ cond_syscall(sys_epoll_wait)
 cond_syscall(sys_pciconfig_read)
 cond_syscall(sys_pciconfig_write)
 
+/* arch-specific weak syscall entries */
+cond_syscall(sys_pciconfig_iobase)
+
 static int set_one_prio(struct task_struct *p, int niceval, int error)
 {
 	int no_nice;

_

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [patch] arch-specific cond_syscall usage issues

[Andrew Morton]

Matt Mackall <mpm@selenic.com> wrote:
>
>  Experimenting with trying to use cond_syscall for a few arch-specific
>  syscalls, I discovered that it can't actually be used outside the file
>  in which sys_ni_syscall is declared because the assembler doesn't feel
>  obliged to output the symbol in that case:
> 
>  weak.c:
> 
>  #define cond_syscall(x) asm(".weak\t" #x "\n\t.set\t" #x ",sys_ni_syscall");
>  cond_syscall(sys_foo);
> 
>  $ nm weak.o
>           U sys_ni_syscall
> 
>  One arch (PPC) is apparently trying to use cond_syscall this way
>  anyway, though it's probably never been actually tested as the above
>  test was done on a PPC.

So why does the PPC kernel successfully link?

Re: [patch] arch-specific cond_syscall usage issues

[Matt Mackall]

On Fri, Jan 09, 2004 at 07:37:53PM -0800, Andrew Morton wrote:
> Matt Mackall <mpm@selenic.com> wrote:
> >
> >  Experimenting with trying to use cond_syscall for a few arch-specific
> >  syscalls, I discovered that it can't actually be used outside the file
> >  in which sys_ni_syscall is declared because the assembler doesn't feel
> >  obliged to output the symbol in that case:
> > 
> >  weak.c:
> > 
> >  #define cond_syscall(x) asm(".weak\t" #x "\n\t.set\t" #x ",sys_ni_syscall");
> >  cond_syscall(sys_foo);
> > 
> >  $ nm weak.o
> >           U sys_ni_syscall
> > 
> >  One arch (PPC) is apparently trying to use cond_syscall this way
> >  anyway, though it's probably never been actually tested as the above
> >  test was done on a PPC.
> 
> So why does the PPC kernel successfully link?

Presumably because no one's tried it without CONFIG_PCI since this
change went in?

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [patch] arch-specific cond_syscall usage issues

[Pete Zaitcev]

On Fri, 9 Jan 2004 19:37:53 -0800
Andrew Morton <akpm@osdl.org> wrote:

> >  Experimenting with trying to use cond_syscall for a few arch-specific
> >  syscalls, I discovered that it can't actually be used outside the file
> >  in which sys_ni_syscall is declared because the assembler doesn't feel
> >  obliged to output the symbol in that case:

> >  One arch (PPC) is apparently trying to use cond_syscall this way
> >  anyway, though it's probably never been actually tested as the above
> >  test was done on a PPC.
> 
> So why does the PPC kernel successfully link?

Perhaps it never was tested right when the change went in.
I saw the same failure when hch did it to sparc, I later did the same:
moved the syscall into kernel/sys.c. Mine wasn't arch specific.
Maybe 3 arches used it (pci_config_read_word or such).

I saw the way ppc did it at that time (2.5.75),
and thought that their toolchain must have been smarter than mine.

The patch is easy. The hard road would be to take it to binutils people
like H.J.Lu and see what they say.

-- Pete

Re: [patch] arch-specific cond_syscall usage issues

[Matt Mackall]

On Fri, Jan 09, 2004 at 09:21:08PM -0800, Pete Zaitcev wrote:
> On Fri, 9 Jan 2004 19:37:53 -0800
> Andrew Morton <akpm@osdl.org> wrote:
> 
> > >  Experimenting with trying to use cond_syscall for a few arch-specific
> > >  syscalls, I discovered that it can't actually be used outside the file
> > >  in which sys_ni_syscall is declared because the assembler doesn't feel
> > >  obliged to output the symbol in that case:
> 
> > >  One arch (PPC) is apparently trying to use cond_syscall this way
> > >  anyway, though it's probably never been actually tested as the above
> > >  test was done on a PPC.
> > 
> > So why does the PPC kernel successfully link?
> 
> Perhaps it never was tested right when the change went in.

On closer inspection, PPC has this:

config PCI
        bool "PCI support" if 40x || 8260
        default y if !40x && !8260 && !8xx && !APUS
        default PCI_PERMEDIA if !4xx && !8260 && !8xx && APUS
        default PCI_QSPAN if !4xx && !8260 && 8xx

which suggests that non-PCI PPC are limited to very old and/or
embedded boxes. And indeed compiling it for one of these (...much time
elapses...) gets us:

arch/ppc/kernel/built-in.o(.data+0x380):arch/ppc/kernel/entry.S:
undefined reference to `sys_pciconfig_iobase'

> The patch is easy. The hard road would be to take it to binutils people
> like H.J.Lu and see what they say.

I believe Dave M mentioned that gcc uses weak symbols similarly, so
they've probably decided that the necessary smarts to do what we
originally wanted are too much trouble.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [patch] arch-specific cond_syscall usage issues

[Tom Rini]

On Fri, Jan 09, 2004 at 07:37:53PM -0800, Andrew Morton wrote:

> Matt Mackall <mpm@selenic.com> wrote:
> >
> >  Experimenting with trying to use cond_syscall for a few arch-specific
> >  syscalls, I discovered that it can't actually be used outside the file
> >  in which sys_ni_syscall is declared because the assembler doesn't feel
> >  obliged to output the symbol in that case:
> > 
> >  weak.c:
> > 
> >  #define cond_syscall(x) asm(".weak\t" #x "\n\t.set\t" #x ",sys_ni_syscall");
> >  cond_syscall(sys_foo);
> > 
> >  $ nm weak.o
> >           U sys_ni_syscall
> > 
> >  One arch (PPC) is apparently trying to use cond_syscall this way
> >  anyway, though it's probably never been actually tested as the above
> >  test was done on a PPC.
> 
> So why does the PPC kernel successfully link?

Since this looks to have missed -mm3, I'm going to follow up here (I
hate playing catch-up sometimes).

As has been previously noted, the cond_syscall is only ever cared about
on PPC when you try for !PCI.  And this only happens realistically now,
on MPC8xx (it's usually present on IBM 4xx, and lets ignore APUS).
MPC8xx support has been broken for a while, but hopefully will get fixed
'soon'.

So can we please move this cond_syscall into kernel/sys.c ?

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [patch] arch-specific cond_syscall usage issues

[Andrew Morton]

Tom Rini <trini@kernel.crashing.org> wrote:
>
>  As has been previously noted, the cond_syscall is only ever cared about
>  on PPC when you try for !PCI.  And this only happens realistically now,
>  on MPC8xx (it's usually present on IBM 4xx, and lets ignore APUS).
>  MPC8xx support has been broken for a while, but hopefully will get fixed
>  'soon'.
> 
>  So can we please move this cond_syscall into kernel/sys.c ?

Spose so.  Are we sure it shouldn't be inside soem ppc-specfic ifdef?

Re: [patch] arch-specific cond_syscall usage issues

[Matt Mackall]

On Wed, Jan 14, 2004 at 11:31:07AM -0800, Andrew Morton wrote:
> Tom Rini <trini@kernel.crashing.org> wrote:
> >
> >  As has been previously noted, the cond_syscall is only ever cared about
> >  on PPC when you try for !PCI.  And this only happens realistically now,
> >  on MPC8xx (it's usually present on IBM 4xx, and lets ignore APUS).
> >  MPC8xx support has been broken for a while, but hopefully will get fixed
> >  'soon'.
> > 
> >  So can we please move this cond_syscall into kernel/sys.c ?
> 
> Spose so.  Are we sure it shouldn't be inside soem ppc-specfic ifdef?

Probably not worth the trouble as it's a weak symbol - it can't
interfere with anything. In the unlikely case that some other arch
intends to define it and fails to, well, they'll get sys_ni_syscall
instead. By that logic, I've done the same with some of the stuff I've
made conditional for x86, like sys_vm86 and the like.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Re: [patch] arch-specific cond_syscall usage issues

[Tom Rini]

On Wed, Jan 14, 2004 at 11:31:07AM -0800, Andrew Morton wrote:

> Tom Rini <trini@kernel.crashing.org> wrote:
> >
> >  As has been previously noted, the cond_syscall is only ever cared about
> >  on PPC when you try for !PCI.  And this only happens realistically now,
> >  on MPC8xx (it's usually present on IBM 4xx, and lets ignore APUS).
> >  MPC8xx support has been broken for a while, but hopefully will get fixed
> >  'soon'.
> > 
> >  So can we please move this cond_syscall into kernel/sys.c ?
> 
> Spose so.  Are we sure it shouldn't be inside soem ppc-specfic ifdef?

At an extreme space concern it could be covered in a PPC32 || ALPHA
test.  It should do no harm if it's not.

-- 
Tom Rini
http://gate.crashing.org/~trini/

[PATCH 2.6] include/asm-ppc/dma-mapping.h: dma_unmap_page()

[Arthur Othieno]

Hi,

Duplicate definition of dma_unmap_single() should actually be
dma_unmap_page().

Against 2.6.6-rc2. Thanks.


 dma-mapping.h |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


--- a/include/asm-ppc/dma-mapping.h	2004-04-11 14:05:45.000000000 +0200
+++ b/include/asm-ppc/dma-mapping.h	2004-04-22 18:06:53.000000000 +0200
@@ -77,7 +77,7 @@ dma_map_page(struct device *dev, struct 
 }
 
 /* We do nothing. */
-#define dma_unmap_single(dev, addr, size, dir)	do { } while (0)
+#define dma_unmap_page(dev, addr, size, dir)	do { } while (0)
 
 static inline int
 dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,

Re: [PATCH 2.6] include/asm-ppc/dma-mapping.h: dma_unmap_page()

[Tom Rini]

[ Playing catchup ]

On Fri, Apr 23, 2004 at 09:14:29PM +0200, Arthur Othieno wrote:

> Hi,
> 
> Duplicate definition of dma_unmap_single() should actually be
> dma_unmap_page().
> 
> Against 2.6.6-rc2. Thanks.
> 
> 
>  dma-mapping.h |    2 +-
>  1 files changed, 1 insertion(+), 1 deletion(-)
> 
> 
> --- a/include/asm-ppc/dma-mapping.h	2004-04-11 14:05:45.000000000 +0200
> +++ b/include/asm-ppc/dma-mapping.h	2004-04-22 18:06:53.000000000 +0200
> @@ -77,7 +77,7 @@ dma_map_page(struct device *dev, struct 
>  }
>  
>  /* We do nothing. */
> -#define dma_unmap_single(dev, addr, size, dir)	do { } while (0)
> +#define dma_unmap_page(dev, addr, size, dir)	do { } while (0)
>  
>  static inline int
>  dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,

If this hasn't made it in already, it would appear to be correct and
should go in.

-- 
Tom Rini
http://gate.crashing.org/~trini/