Yet another fix

Yet another fix

[ilya]

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

I am not sure this is correct solution to a problem. Or rather, I'm pretty
sure it is incorrect one.. There is a reference to module_map somewhere, however
it is not inculded if modules are disabled. Here is sorta fix

Index: include/asm-mips64/module.h
===================================================================
RCS file: /home/cvs/linux/include/asm-mips64/module.h,v
retrieving revision 1.5
diff -u -r1.5 module.h
--- include/asm-mips64/module.h 1 Jun 2003 00:39:15 -0000       1.5
+++ include/asm-mips64/module.h 2 Jun 2003 03:59:23 -0000
@@ -11,4 +11,8 @@
 #define Elf_Sym Elf32_Sym
 #define Elf_Ehdr Elf32_Ehdr
 
+#ifndef CONFIG_MODULES
+#define module_map(x) vmalloc(x)
+#endif
+
 #endif /* _ASM_MODULE_H */


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

Re: Yet another fix

[Keith Owens]

On Sun, 1 Jun 2003 21:14:24 -0700, 
ilya@theIlya.com wrote:
>I am not sure this is correct solution to a problem. Or rather, I'm pretty
>sure it is incorrect one.. There is a reference to module_map somewhere, however
>it is not inculded if modules are disabled. Here is sorta fix
>
>Index: include/asm-mips64/module.h
>===================================================================
>RCS file: /home/cvs/linux/include/asm-mips64/module.h,v
>retrieving revision 1.5
>diff -u -r1.5 module.h
>--- include/asm-mips64/module.h 1 Jun 2003 00:39:15 -0000       1.5
>+++ include/asm-mips64/module.h 2 Jun 2003 03:59:23 -0000
>@@ -11,4 +11,8 @@
> #define Elf_Sym Elf32_Sym
> #define Elf_Ehdr Elf32_Ehdr
> 
>+#ifndef CONFIG_MODULES
>+#define module_map(x) vmalloc(x)
>+#endif
>+
> #endif /* _ASM_MODULE_H */

That fix is incorrect.  There should be no references to module_map
when CONFIG_MODULES=n.  Please find out where module_map is being
incorrectly used and fix that code.

Re: Yet another fix

[ilya]

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

module_map is referenced in register_ioctl32_conversion in arch/mips64/ioctl32.c
As far as I can see, it should simply be possible to replace module_map
with vmalloc in there, but I am not sure, as I don't know how exactly
ioctl translations work...

On Mon, Jun 02, 2003 at 02:49:24PM +1000, Keith Owens wrote:
> On Sun, 1 Jun 2003 21:14:24 -0700, 
> ilya@theIlya.com wrote:
> >I am not sure this is correct solution to a problem. Or rather, I'm pretty
> >sure it is incorrect one.. There is a reference to module_map somewhere, however
> >it is not inculded if modules are disabled. Here is sorta fix
> >
> >Index: include/asm-mips64/module.h
> >===================================================================
> >RCS file: /home/cvs/linux/include/asm-mips64/module.h,v
> >retrieving revision 1.5
> >diff -u -r1.5 module.h
> >--- include/asm-mips64/module.h 1 Jun 2003 00:39:15 -0000       1.5
> >+++ include/asm-mips64/module.h 2 Jun 2003 03:59:23 -0000
> >@@ -11,4 +11,8 @@
> > #define Elf_Sym Elf32_Sym
> > #define Elf_Ehdr Elf32_Ehdr
> > 
> >+#ifndef CONFIG_MODULES
> >+#define module_map(x) vmalloc(x)
> >+#endif
> >+
> > #endif /* _ASM_MODULE_H */
> 
> That fix is incorrect.  There should be no references to module_map
> when CONFIG_MODULES=n.  Please find out where module_map is being
> incorrectly used and fix that code.
> 

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

Re: Yet another fix

[Keith Owens]

On Sun, 1 Jun 2003 21:57:00 -0700, 
ilya@theIlya.com wrote:
>module_map is referenced in register_ioctl32_conversion in arch/mips64/ioctl32.c
>As far as I can see, it should simply be possible to replace module_map
>with vmalloc in there, but I am not sure, as I don't know how exactly
>ioctl translations work...

Not in 2.4.20 nor 2.4.21-rc6 from Marcelo, must be a mips local change.
I strongly suggest that you get rid of it, there is no good reason to
emulate the 32 bit module syscalls on a 64 bit machine.  modutils is
pure Linux and there is absolutely no justification for emulating 32
bit versions of modutils when the user can install the 64 bit version
of modutils instead.  32 bit emulation is a crutch to let binary only
programs work when you do not have the source to rebuild to 64 bit, by
definition we have the source to modutils.

IA64 and x86_64 make no attempt to emulate 32 bit modutils.  sparc64,
ppc64 and s390x all pass the data straight to the 64 bit kernel code,
they require the user space modutils to supply 64 bit data.  Emulation
is a waste of time.

Re: Yet another fix

[Ralf Baechle]

On Mon, Jun 02, 2003 at 11:27:46PM +1000, Keith Owens wrote:

> Not in 2.4.20 nor 2.4.21-rc6 from Marcelo, must be a mips local change.
> I strongly suggest that you get rid of it, there is no good reason to
> emulate the 32 bit module syscalls on a 64 bit machine.  modutils is
> pure Linux and there is absolutely no justification for emulating 32
> bit versions of modutils when the user can install the 64 bit version
> of modutils instead.  32 bit emulation is a crutch to let binary only
> programs work when you do not have the source to rebuild to 64 bit, by
> definition we have the source to modutils.

Until very recently there was no 64-bit userland.

> IA64 and x86_64 make no attempt to emulate 32 bit modutils.  sparc64,
> ppc64 and s390x all pass the data straight to the 64 bit kernel code,
> they require the user space modutils to supply 64 bit data.  Emulation
> is a waste of time.

The code simply does the sparc64 thing.  Heck, it is the sparc64 code
with minor changes.

  Ralf

Re: Yet another fix

[ilya]

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

For starters, I'm talking about 2.5.51 here.
Secondly, what does register_ioctl32_conversion have to do with
emulating 32bit modutils? Code in quesion is this:
int register_ioctl32_conversion(unsigned int cmd, int (*handler)(unsigned int, unsigned int, unsigned long, ...))
{
	int i;
	if (!additional_ioctls) {
		additional_ioctls = module_map(PAGE_SIZE);
		if (!additional_ioctls)
			return -ENOMEM;
		memset(additional_ioctls, 0, PAGE_SIZE);
	}
.....

As far as I can tell, There is nothing that prevents us from
replacing module_map with vmalloc, or even get_free_pages,
but I am not sure. There must be some reason, why it is there :)
Ralf, it's question for you.

	Ilya.


On Mon, Jun 02, 2003 at 11:27:46PM +1000, Keith Owens wrote:
> On Sun, 1 Jun 2003 21:57:00 -0700, 
> ilya@theIlya.com wrote:
> >module_map is referenced in register_ioctl32_conversion in arch/mips64/ioctl32.c
> >As far as I can see, it should simply be possible to replace module_map
> >with vmalloc in there, but I am not sure, as I don't know how exactly
> >ioctl translations work...
> 
> Not in 2.4.20 nor 2.4.21-rc6 from Marcelo, must be a mips local change.
> I strongly suggest that you get rid of it, there is no good reason to
> emulate the 32 bit module syscalls on a 64 bit machine.  modutils is
> pure Linux and there is absolutely no justification for emulating 32
> bit versions of modutils when the user can install the 64 bit version
> of modutils instead.  32 bit emulation is a crutch to let binary only
> programs work when you do not have the source to rebuild to 64 bit, by
> definition we have the source to modutils.
> 
> IA64 and x86_64 make no attempt to emulate 32 bit modutils.  sparc64,
> ppc64 and s390x all pass the data straight to the 64 bit kernel code,
> they require the user space modutils to supply 64 bit data.  Emulation
> is a waste of time.
> 

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

Re: Yet another fix

[Keith Owens]

On Mon, 2 Jun 2003 07:30:23 -0700, 
ilya@theIlya.com wrote:
>For starters, I'm talking about 2.5.51 here.

Sorry,  thought you were talking about 2.4 kernels.

>Secondly, what does register_ioctl32_conversion have to do with
>emulating 32bit modutils?

See above, I thought this was 2.4.

>Code in quesion is this:
>int register_ioctl32_conversion(unsigned int cmd, int (*handler)(unsigned i=
>nt, unsigned int, unsigned long, ...))
>{
>	int i;
>	if (!additional_ioctls) {
>		additional_ioctls =3D module_map(PAGE_SIZE);
>		if (!additional_ioctls)
>			return -ENOMEM;
>		memset(additional_ioctls, 0, PAGE_SIZE);
>	}

I cannot find register_ioctl32_conversion in 2.5.51.

>As far as I can tell, There is nothing that prevents us from
>replacing module_map with vmalloc, or even get_free_pages,
>but I am not sure. There must be some reason, why it is there :)
>Ralf, it's question for you.

Without seeing the code that uses register_ioctl32_conversion (it is
not in Linus's kernel), I would be guessing.  But I suspect that you
are right, it should use vmalloc.