Re: [PATCH 14/19] OpenRISC: Module support

Re: [PATCH 14/19] OpenRISC: Module support

[Arnd Bergmann]

On Tuesday 21 June 2011, Valdis.Kletnieks@vt.edu wrote:
> On Sun, 19 Jun 2011 13:43:40 +0200, Jonas Bonn said:

> > diff --git a/arch/openrisc/kernel/module.c b/arch/openrisc/kernel/module.c
> > new file mode 100644
> > index 0000000..952b129
> > --- /dev/null
> > +++ b/arch/openrisc/kernel/module.c
> > @@ -0,0 +1,126 @@
> > +/*
> > + * OpenRISC module.c
> 
> Lose the filenames in the headers - you do this in multiple files.

Good point.
 
> Also, do you even *need* a module.c? There isn't much in here, will the main
> kernel/module.c do the work needed?  Or did you end up with one because the
> arch you cloned had one?

Right now you need a module.c in each architecture defining all these
functions. However, I think it would be cool if we didn't need this
all of them. In case of openrisc, the architecture should only have
to define a apply_relocate_add() function.

Jonas, could you add a default implementation for each of the functions
for include/linux/moduleloader.h, so that you only have to provide the
one function?

I think that an easy way to do that would be to add to kernel/module.c
code like:

#ifndef module_alloc
void *module_alloc(unsigned long size)
{
	return vmalloc(size);
}
#endif

Then each architecture that needs a special version can do
#define module_alloc(size) module_alloc(size)
in their respective asm/module.h

	Arnd

Re: [PATCH 14/19] OpenRISC: Module support

[Arnd Bergmann]

On Tuesday 21 June 2011, Valdis.Kletnieks@vt.edu wrote:
> On Sun, 19 Jun 2011 13:43:40 +0200, Jonas Bonn said:

> > diff --git a/arch/openrisc/kernel/module.c b/arch/openrisc/kernel/module.c
> > new file mode 100644
> > index 0000000..952b129
> > --- /dev/null
> > +++ b/arch/openrisc/kernel/module.c
> > @@ -0,0 +1,126 @@
> > +/*
> > + * OpenRISC module.c
> 
> Lose the filenames in the headers - you do this in multiple files.

Good point.
 
> Also, do you even *need* a module.c? There isn't much in here, will the main
> kernel/module.c do the work needed?  Or did you end up with one because the
> arch you cloned had one?

Right now you need a module.c in each architecture defining all these
functions. However, I think it would be cool if we didn't need this
all of them. In case of openrisc, the architecture should only have
to define a apply_relocate_add() function.

Jonas, could you add a default implementation for each of the functions
for include/linux/moduleloader.h, so that you only have to provide the
one function?

I think that an easy way to do that would be to add to kernel/module.c
code like:

#ifndef module_alloc
void *module_alloc(unsigned long size)
{
	return vmalloc(size);
}
#endif

Then each architecture that needs a special version can do
#define module_alloc(size) module_alloc(size)
in their respective asm/module.h

	Arnd

[PATCH 1/1] Add default implementations for moduleloader hooks

[Jonas Bonn]

This patch puts in place generic implementations for the moduleloader
hooks that can be used by architectures that don't need to do anything
fancy for module loading.

As an example, the OpenRISC architecture is modified to use these
generic hooks.

This patch is just for comment for now... will need to add the necessary
definitions to the other architectures if this deemed ok.

Verified for OpenRISC... no sign-off for now.
---

Hi Arnd,

Threw this together to check if this is what you had in mind... seems pretty
good to me.  Will have to do the cleanups in the other architectures, but
thought I'd post this in response to your suggestion first.  What do you think?

/Jonas

 arch/openrisc/include/asm/module.h |   31 +++++++++++++++++
 arch/openrisc/kernel/module.c      |   62 ++---------------------------------
 kernel/module.c                    |   63 ++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 58 deletions(-)
 create mode 100644 arch/openrisc/include/asm/module.h

diff --git a/arch/openrisc/include/asm/module.h b/arch/openrisc/include/asm/module.h
new file mode 100644
index 0000000..c550bed
--- /dev/null
+++ b/arch/openrisc/include/asm/module.h
@@ -0,0 +1,31 @@
+/*
+ * OpenRISC Linux
+ *
+ * Linux architectural port borrowing liberally from similar works of
+ * others.  All original copyrights apply as per the original source
+ * declaration.
+ *
+ * OpenRISC implementation:
+ * Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __ASM_OPENRISC_MODULE_H__
+#define __ASM_OPENRISC_MODULE_H__
+
+/*
+ * include/linux/moduleloader.h provides a set of architecture specific hooks
+ * for the module loader.  There are trivial default implementations for
+ * all of these hooks, but we need to set up some definitions for those hooks
+ * that we need to override.
+ */
+
+#define apply_relocate_add apply_relocate_add
+
+#include <asm-generic/module.h>
+
+#endif
diff --git a/arch/openrisc/kernel/module.c b/arch/openrisc/kernel/module.c
index 952b129..6a35c86 100644
--- a/arch/openrisc/kernel/module.c
+++ b/arch/openrisc/kernel/module.c
@@ -16,49 +16,16 @@
 
 #include <linux/moduleloader.h>
 #include <linux/elf.h>
-#include <linux/vmalloc.h>
-#include <linux/fs.h>
-#include <linux/string.h>
-#include <linux/kernel.h>
-
-void *module_alloc(unsigned long size)
-{
-	pr_debug("module_alloc size: %lu\n", size);
-
-	if (size == 0)
-		return NULL;
-
-	return vmalloc(size);
-}
-
-
-/* Free memory returned from module_alloc */
-void module_free(struct module *mod, void *module_region)
-{
-	vfree(module_region);
-	/* FIXME: If module_region == mod->init_region, trim exception
-           table entries. */
-}
-
-/* We don't need anything special. */
-int module_frob_arch_sections(Elf_Ehdr *hdr,
-			      Elf_Shdr *sechdrs,
-			      char *secstrings,
-			      struct module *mod)
-{
-	return 0;
-}
 
 int apply_relocate_add(Elf32_Shdr *sechdrs,
-		       const char *strtab,
-		       unsigned int symindex,
-		       unsigned int relsec,
-		       struct module *me)
+                       const char *strtab,
+                       unsigned int symindex,
+                       unsigned int relsec,
+                       struct module *me)
 {
 	unsigned int i;
 	Elf32_Rela *rel = (void *)sechdrs[relsec].sh_addr;
 	Elf32_Sym *sym;
-//	Elf32_Addr relocation;
 	uint32_t *location;
 	uint32_t value;
 
@@ -103,24 +70,3 @@ int apply_relocate_add(Elf32_Shdr *sechdrs,
 
 	return 0;
 }
-
-int apply_relocate(Elf32_Shdr *sechdrs,
-		   const char *strtab,
-		   unsigned int symindex,
-		   unsigned int relsec,
-		   struct module *me)
-{
-	pr_err("module %s: REL relocation unsupported\n", me->name);
-	return -ENOEXEC;
-}
-
-int module_finalize(const Elf_Ehdr *hdr,
-		    const Elf_Shdr *sechdrs,
-		    struct module *me)
-{
-	return 0;
-}
-
-void module_arch_cleanup(struct module *mod)
-{
-}
diff --git a/kernel/module.c b/kernel/module.c
index 795bdc7..ecb5538 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1697,6 +1697,19 @@ static void unset_module_core_ro_nx(struct module *mod) { }
 static void unset_module_init_ro_nx(struct module *mod) { }
 #endif
 
+#ifndef module_free
+void module_free(struct module *mod, void *module_region)
+{
+	vfree(module_region);
+}
+#endif
+
+#ifndef module_arch_cleanup
+void module_arch_cleanup(struct module *mod)
+{
+}
+#endif
+
 /* Free a module, remove from lists, etc. */
 static void free_module(struct module *mod)
 {
@@ -1851,6 +1864,30 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 	return ret;
 }
 
+#ifndef apply_relocate
+int apply_relocate(Elf32_Shdr *sechdrs,
+                   const char *strtab,
+                   unsigned int symindex,
+                   unsigned int relsec,
+                   struct module *me)
+{
+	pr_err("module %s: REL relocation unsupported\n", me->name);
+	return -ENOEXEC;
+}
+#endif
+
+#ifndef apply_relocate_add
+int apply_relocate_add(Elf32_Shdr *sechdrs,
+                   const char *strtab,
+                   unsigned int symindex,
+                   unsigned int relsec,
+                   struct module *me)
+{
+	pr_err("module %s: RELA relocation unsupported\n", me->name);
+	return -ENOEXEC;
+}
+#endif
+
 static int apply_relocations(struct module *mod, const struct load_info *info)
 {
 	unsigned int i;
@@ -2235,6 +2272,13 @@ static void dynamic_debug_remove(struct _ddebug *debug)
 		ddebug_remove_module(debug->modname);
 }
 
+#ifndef module_alloc
+void *module_alloc(unsigned long size)
+{
+	return size == 0 ? NULL : vmalloc(size);
+}
+#endif
+
 static void *module_alloc_update_bounds(unsigned long size)
 {
 	void *ret = module_alloc(size);
@@ -2645,6 +2689,16 @@ static void flush_module_icache(const struct module *mod)
 	set_fs(old_fs);
 }
 
+#ifndef module_frob_arch_sections
+int module_frob_arch_sections(Elf_Ehdr *hdr,
+                              Elf_Shdr *sechdrs,
+                              char *secstrings,
+                              struct module *mod)
+{
+	return 0;
+}
+#endif
+
 static struct module *layout_and_allocate(struct load_info *info)
 {
 	/* Module within temporary copy. */
@@ -2716,6 +2770,15 @@ static void module_deallocate(struct module *mod, struct load_info *info)
 	module_free(mod, mod->module_core);
 }
 
+#ifndef module_finalize
+int module_finalize(const Elf_Ehdr *hdr,
+                    const Elf_Shdr *sechdrs,
+                    struct module *me)
+{
+	return 0;
+}
+#endif
+
 static int post_relocation(struct module *mod, const struct load_info *info)
 {
 	/* Sort exception table now relocations are done. */
-- 
1.7.4.1

[PATCH 1/1] Add default implementations for moduleloader hooks

[Jonas Bonn]

This patch puts in place generic implementations for the moduleloader
hooks that can be used by architectures that don't need to do anything
fancy for module loading.

As an example, the OpenRISC architecture is modified to use these
generic hooks.

This patch is just for comment for now... will need to add the necessary
definitions to the other architectures if this deemed ok.

Verified for OpenRISC... no sign-off for now.
---

Hi Arnd,

Threw this together to check if this is what you had in mind... seems pretty
good to me.  Will have to do the cleanups in the other architectures, but
thought I'd post this in response to your suggestion first.  What do you think?

/Jonas

 arch/openrisc/include/asm/module.h |   31 +++++++++++++++++
 arch/openrisc/kernel/module.c      |   62 ++---------------------------------
 kernel/module.c                    |   63 ++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 58 deletions(-)
 create mode 100644 arch/openrisc/include/asm/module.h

diff --git a/arch/openrisc/include/asm/module.h b/arch/openrisc/include/asm/module.h
new file mode 100644
index 0000000..c550bed
--- /dev/null
+++ b/arch/openrisc/include/asm/module.h
@@ -0,0 +1,31 @@
+/*
+ * OpenRISC Linux
+ *
+ * Linux architectural port borrowing liberally from similar works of
+ * others.  All original copyrights apply as per the original source
+ * declaration.
+ *
+ * OpenRISC implementation:
+ * Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __ASM_OPENRISC_MODULE_H__
+#define __ASM_OPENRISC_MODULE_H__
+
+/*
+ * include/linux/moduleloader.h provides a set of architecture specific hooks
+ * for the module loader.  There are trivial default implementations for
+ * all of these hooks, but we need to set up some definitions for those hooks
+ * that we need to override.
+ */
+
+#define apply_relocate_add apply_relocate_add
+
+#include <asm-generic/module.h>
+
+#endif
diff --git a/arch/openrisc/kernel/module.c b/arch/openrisc/kernel/module.c
index 952b129..6a35c86 100644
--- a/arch/openrisc/kernel/module.c
+++ b/arch/openrisc/kernel/module.c
@@ -16,49 +16,16 @@
 
 #include <linux/moduleloader.h>
 #include <linux/elf.h>
-#include <linux/vmalloc.h>
-#include <linux/fs.h>
-#include <linux/string.h>
-#include <linux/kernel.h>
-
-void *module_alloc(unsigned long size)
-{
-	pr_debug("module_alloc size: %lu\n", size);
-
-	if (size == 0)
-		return NULL;
-
-	return vmalloc(size);
-}
-
-
-/* Free memory returned from module_alloc */
-void module_free(struct module *mod, void *module_region)
-{
-	vfree(module_region);
-	/* FIXME: If module_region == mod->init_region, trim exception
-           table entries. */
-}
-
-/* We don't need anything special. */
-int module_frob_arch_sections(Elf_Ehdr *hdr,
-			      Elf_Shdr *sechdrs,
-			      char *secstrings,
-			      struct module *mod)
-{
-	return 0;
-}
 
 int apply_relocate_add(Elf32_Shdr *sechdrs,
-		       const char *strtab,
-		       unsigned int symindex,
-		       unsigned int relsec,
-		       struct module *me)
+                       const char *strtab,
+                       unsigned int symindex,
+                       unsigned int relsec,
+                       struct module *me)
 {
 	unsigned int i;
 	Elf32_Rela *rel = (void *)sechdrs[relsec].sh_addr;
 	Elf32_Sym *sym;
-//	Elf32_Addr relocation;
 	uint32_t *location;
 	uint32_t value;
 
@@ -103,24 +70,3 @@ int apply_relocate_add(Elf32_Shdr *sechdrs,
 
 	return 0;
 }
-
-int apply_relocate(Elf32_Shdr *sechdrs,
-		   const char *strtab,
-		   unsigned int symindex,
-		   unsigned int relsec,
-		   struct module *me)
-{
-	pr_err("module %s: REL relocation unsupported\n", me->name);
-	return -ENOEXEC;
-}
-
-int module_finalize(const Elf_Ehdr *hdr,
-		    const Elf_Shdr *sechdrs,
-		    struct module *me)
-{
-	return 0;
-}
-
-void module_arch_cleanup(struct module *mod)
-{
-}
diff --git a/kernel/module.c b/kernel/module.c
index 795bdc7..ecb5538 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1697,6 +1697,19 @@ static void unset_module_core_ro_nx(struct module *mod) { }
 static void unset_module_init_ro_nx(struct module *mod) { }
 #endif
 
+#ifndef module_free
+void module_free(struct module *mod, void *module_region)
+{
+	vfree(module_region);
+}
+#endif
+
+#ifndef module_arch_cleanup
+void module_arch_cleanup(struct module *mod)
+{
+}
+#endif
+
 /* Free a module, remove from lists, etc. */
 static void free_module(struct module *mod)
 {
@@ -1851,6 +1864,30 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
 	return ret;
 }
 
+#ifndef apply_relocate
+int apply_relocate(Elf32_Shdr *sechdrs,
+                   const char *strtab,
+                   unsigned int symindex,
+                   unsigned int relsec,
+                   struct module *me)
+{
+	pr_err("module %s: REL relocation unsupported\n", me->name);
+	return -ENOEXEC;
+}
+#endif
+
+#ifndef apply_relocate_add
+int apply_relocate_add(Elf32_Shdr *sechdrs,
+                   const char *strtab,
+                   unsigned int symindex,
+                   unsigned int relsec,
+                   struct module *me)
+{
+	pr_err("module %s: RELA relocation unsupported\n", me->name);
+	return -ENOEXEC;
+}
+#endif
+
 static int apply_relocations(struct module *mod, const struct load_info *info)
 {
 	unsigned int i;
@@ -2235,6 +2272,13 @@ static void dynamic_debug_remove(struct _ddebug *debug)
 		ddebug_remove_module(debug->modname);
 }
 
+#ifndef module_alloc
+void *module_alloc(unsigned long size)
+{
+	return size == 0 ? NULL : vmalloc(size);
+}
+#endif
+
 static void *module_alloc_update_bounds(unsigned long size)
 {
 	void *ret = module_alloc(size);
@@ -2645,6 +2689,16 @@ static void flush_module_icache(const struct module *mod)
 	set_fs(old_fs);
 }
 
+#ifndef module_frob_arch_sections
+int module_frob_arch_sections(Elf_Ehdr *hdr,
+                              Elf_Shdr *sechdrs,
+                              char *secstrings,
+                              struct module *mod)
+{
+	return 0;
+}
+#endif
+
 static struct module *layout_and_allocate(struct load_info *info)
 {
 	/* Module within temporary copy. */
@@ -2716,6 +2770,15 @@ static void module_deallocate(struct module *mod, struct load_info *info)
 	module_free(mod, mod->module_core);
 }
 
+#ifndef module_finalize
+int module_finalize(const Elf_Ehdr *hdr,
+                    const Elf_Shdr *sechdrs,
+                    struct module *me)
+{
+	return 0;
+}
+#endif
+
 static int post_relocation(struct module *mod, const struct load_info *info)
 {
 	/* Sort exception table now relocations are done. */
-- 
1.7.4.1

Re: [PATCH 14/19] OpenRISC: Module support

[Jonas Bonn]

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

On Wed, 2011-06-22 at 16:26 +0200, Arnd Bergmann wrote:
> On Tuesday 21 June 2011, Valdis.Kletnieks@vt.edu wrote:
> > On Sun, 19 Jun 2011 13:43:40 +0200, Jonas Bonn said:
> 
> > > diff --git a/arch/openrisc/kernel/module.c b/arch/openrisc/kernel/module.c
> > > new file mode 100644
> > > index 0000000..952b129
> > > --- /dev/null
> > > +++ b/arch/openrisc/kernel/module.c
> > > @@ -0,0 +1,126 @@
> > > +/*
> > > + * OpenRISC module.c
> > 
> > Lose the filenames in the headers - you do this in multiple files.
> 
> Good point.

What's an acceptable "comment title"?  Most other arch's seem to put the
filename there... I understand this is undesirable, but what can I put
there instead?  Is "OpenRISC Linux" acceptable, or is nothing at all
preferable?

Just want to be clear about what's allowed before I go changing all of
these...

/Jonas


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

Re: [PATCH 14/19] OpenRISC: Module support

[Arnd Bergmann]

On Wednesday 22 June 2011 21:14:10 Jonas Bonn wrote:
> What's an acceptable "comment title"?  Most other arch's seem to put the
> filename there... I understand this is undesirable, but what can I put
> there instead?  Is "OpenRISC Linux" acceptable, or is nothing at all
> preferable?
> 
> Just want to be clear about what's allowed before I go changing all of
> these...

You don't really need anything in the comment, if you want to put something
there, I'd recommend a one-line summary of what the file is used for.

	Arnd

Re: [PATCH 14/19] OpenRISC: Module support

[Jonas Bonn]

On Wed, 2011-06-22 at 21:58 +0200, Arnd Bergmann wrote:
> On Wednesday 22 June 2011 21:14:10 Jonas Bonn wrote:
> > What's an acceptable "comment title"?  Most other arch's seem to put the
> > filename there... I understand this is undesirable, but what can I put
> > there instead?  Is "OpenRISC Linux" acceptable, or is nothing at all
> > preferable?
> > 
> > Just want to be clear about what's allowed before I go changing all of
> > these...
> 
> You don't really need anything in the comment, if you want to put something
> there, I'd recommend a one-line summary of what the file is used for.
> 

No license information, copyright, etc?  Nothing?  Would it be
preferable to delete the comments altogether...?

/Jonas

Re: [PATCH 14/19] OpenRISC: Module support

[Arnd Bergmann]

On Wednesday 22 June 2011 22:05:39 Jonas Bonn wrote:
> On Wed, 2011-06-22 at 21:58 +0200, Arnd Bergmann wrote:
> > On Wednesday 22 June 2011 21:14:10 Jonas Bonn wrote:
> > > What's an acceptable "comment title"?  Most other arch's seem to put the
> > > filename there... I understand this is undesirable, but what can I put
> > > there instead?  Is "OpenRISC Linux" acceptable, or is nothing at all
> > > preferable?
> > > 
> > > Just want to be clear about what's allowed before I go changing all of
> > > these...
> > 
> > You don't really need anything in the comment, if you want to put something
> > there, I'd recommend a one-line summary of what the file is used for.
> > 
> 
> No license information, copyright, etc?  Nothing?  Would it be
> preferable to delete the comments altogether...?

A lot of people like to have license and copyright information in there,
but it's your choice when you wrote the file.

However, you should never remove someone else's copyright statement when
the code written by that person remain, and you can not change the
license without the author's permission, at least not to an incompatible
one.

	Arnd

Re: [PATCH 14/19] OpenRISC: Module support

[Jonas Bonn]

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

On Wed, 2011-06-22 at 16:26 +0200, Arnd Bergmann wrote:
> I think that an easy way to do that would be to add to kernel/module.c
> code like:
> 
> #ifndef module_alloc
> void *module_alloc(unsigned long size)
> {
> 	return vmalloc(size);
> }
> #endif
> 

I noticed that kernel/module.c already has this:

unsigned int __weak arch_mod_section_prepend(struct module *mod,
                                             unsigned int section)


Is using a __weak attribute on the default (generic) implementations a
better approach?

> Then each architecture that needs a special version can do
> #define module_alloc(size) module_alloc(size)
> in their respective asm/module.h

With the __weak variant this wouldn't be necessary...

/Jonas

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

Re: [PATCH 14/19] OpenRISC: Module support

[Jonas Bonn]

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

On Wed, 2011-06-22 at 16:26 +0200, Arnd Bergmann wrote:
> I think that an easy way to do that would be to add to kernel/module.c
> code like:
> 
> #ifndef module_alloc
> void *module_alloc(unsigned long size)
> {
> 	return vmalloc(size);
> }
> #endif
> 

I noticed that kernel/module.c already has this:

unsigned int __weak arch_mod_section_prepend(struct module *mod,
                                             unsigned int section)


Is using a __weak attribute on the default (generic) implementations a
better approach?

> Then each architecture that needs a special version can do
> #define module_alloc(size) module_alloc(size)
> in their respective asm/module.h

With the __weak variant this wouldn't be necessary...

/Jonas

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

Re: [PATCH 14/19] OpenRISC: Module support

[Arnd Bergmann]

On Friday 24 June 2011, Jonas Bonn wrote:
> On Wed, 2011-06-22 at 16:26 +0200, Arnd Bergmann wrote:
> > I think that an easy way to do that would be to add to kernel/module.c
> > code like:
> > 
> > #ifndef module_alloc
> > void *module_alloc(unsigned long size)
> > {
> >       return vmalloc(size);
> > }
> > #endif
> > 
> 
> I noticed that kernel/module.c already has this:
> 
> unsigned int __weak arch_mod_section_prepend(struct module *mod,
>                                              unsigned int section)
> 
>
> Is using a __weak attribute on the default (generic) implementations a
> better approach?

I normally don't like using __weak, because it more easily confuses
readers about which version is actually used.

> > Then each architecture that needs a special version can do
> > #define module_alloc(size) module_alloc(size)
> > in their respective asm/module.h
> 
> With the __weak variant this wouldn't be necessary...

Correct. I certainly wouldn't want to see both methods mixed in the
same file. Rusty is maintaining the module loader, maybe he has
a preference.

Rusty, we were discussing the fact that most module loader files
are providing very similar (mostly empty) functions, and that it would
be nice if the new openrisc architecture would only have to provide
apply_relocate_add(), which is actually arch specific, while it could
be changed to use the defaults for everything else.

	Arnd

Re: [PATCH 14/19] OpenRISC: Module support

[Rusty Russell]

On Fri, 24 Jun 2011 12:05:31 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 24 June 2011, Jonas Bonn wrote:
> > I noticed that kernel/module.c already has this:
> > 
> > unsigned int __weak arch_mod_section_prepend(struct module *mod,
> >                                              unsigned int section)
> > 
> >
> > Is using a __weak attribute on the default (generic) implementations a
> > better approach?
> 
> I normally don't like using __weak, because it more easily confuses
> readers about which version is actually used.

I share your reluctance with __weak, but as fewer people want to touch
multiple archs it is becoming the norm.

Would happily accept patches...

Thanks,
Rusty.

Re: [PATCH 14/19] OpenRISC: Module support

[Rusty Russell]

On Fri, 24 Jun 2011 12:05:31 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 24 June 2011, Jonas Bonn wrote:
> > I noticed that kernel/module.c already has this:
> > 
> > unsigned int __weak arch_mod_section_prepend(struct module *mod,
> >                                              unsigned int section)
> > 
> >
> > Is using a __weak attribute on the default (generic) implementations a
> > better approach?
> 
> I normally don't like using __weak, because it more easily confuses
> readers about which version is actually used.

I share your reluctance with __weak, but as fewer people want to touch
multiple archs it is becoming the norm.

Would happily accept patches...

Thanks,
Rusty.