[PATCH v1 00/10] Remove weak function declarations

[Bjorn Helgaas]

A common usage of "weak" is for a default implementation of a function.
An architecture that needs something different can supply a non-weak
("strong") implementation, with the expectation that the linker will select
the strong version and discard the weak default version.

We have a few function declarations in header files annotated as "weak".
That causes every *every* definition to be marked "weak", which means there
is no strong version at all. In this case, the linker selects one of the
weak versions based on link order. I don't think this is what we want.

These patches remove almost all the weak annotations from header files
(MIPS still uses it for get_c0_compare_int(), apparently relying on the
fact that a weak symbol need not be defined at all). In most cases, the
default implementation was already marked weak at the definition. When it
wasn't, I added that.

It might be simplest if I ask Linus to pull these all as a group from my
branch [1]. I'll look for acks from the following people. If I don't see
an ack, I'll drop the patch and you can take it yourself or ignore it as
you wish.

Eric: audit
Thomas, Ingo, or Peter: x86
Ralf: MIPS
John or Thomas: clocksource
Jason: kgdb
Ingo: uprobes
Andrew: vmcore, memory-hotplug

I don't know whether these fix any actual bugs. We *did* have a bug like
this on MIPS a while ago (10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")), so it's possible that they do fix
something.

[1] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=remove-weak-function-declarations

---

Bjorn Helgaas (10):
audit: Remove "weak" from audit_classify_compat_syscall() declaration
x86, intel-mid: Remove "weak" from function declarations
MIPS: CPC: Make mips_cpc_phys_base() static
MIPS: Remove "weak" from platform_maar_init() declaration
MIPS: MT: Move "weak" from vpe_run() declaration to definition
clocksource: Remove "weak" from clocksource_default_clock() declaration
vmcore: Remove "weak" from function declarations
kgdb: Remove "weak" from kgdb_arch_pc() declaration
memory-hotplug: Remove "weak" from memory_block_size_bytes() declaration
uprobes: Remove "weak" from function declarations


arch/mips/include/asm/maar.h | 2 +-
arch/mips/include/asm/mips-cpc.h | 10 ----------
arch/mips/include/asm/vpe.h | 2 +-
arch/mips/kernel/mips-cpc.c | 2 +-
arch/mips/kernel/vpe-mt.c | 2 +-
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
include/linux/audit.h | 2 +-
include/linux/clocksource.h | 2 +-
include/linux/crash_dump.h | 15 +++++++--------
include/linux/kgdb.h | 2 +-
include/linux/memory.h | 2 +-
include/linux/uprobes.h | 14 +++++++-------
12 files changed, 25 insertions(+), 37 deletions(-)

[Bjorn Helgaas]

There's only one audit_classify_compat_syscall() definition, so it doesn't
need to be weak.

Remove the "weak" attribute from the audit_classify_compat_syscall()
declaration.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: AKASHI Takahiro <***@linaro.org>
CC: Richard Guy Briggs <***@redhat.com>
---
include/linux/audit.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 22cfddb75566..1b6098beb46b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -86,7 +86,7 @@ extern unsigned compat_dir_class[];
extern unsigned compat_chattr_class[];
extern unsigned compat_signal_class[];

-extern int __weak audit_classify_compat_syscall(int abi, unsigned syscall);
+extern int audit_classify_compat_syscall(int abi, unsigned syscall);

/* audit_names->type values */
#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */

[Bjorn Helgaas]

[+cc AKASHI, Richard; sorry, I botched my "stg mail" so you weren't
included the first time]

Post by Bjorn Helgaas
There's only one audit_classify_compat_syscall() definition, so it doesn't
need to be weak.
Remove the "weak" attribute from the audit_classify_compat_syscall()
declaration.
---
include/linux/audit.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 22cfddb75566..1b6098beb46b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -86,7 +86,7 @@ extern unsigned compat_dir_class[];
extern unsigned compat_chattr_class[];
extern unsigned compat_signal_class[];
-extern int __weak audit_classify_compat_syscall(int abi, unsigned syscall);
+extern int audit_classify_compat_syscall(int abi, unsigned syscall);
/* audit_names->type values */
#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */

[Richard Guy Briggs]

Post by Bjorn Helgaas
[+cc AKASHI, Richard; sorry, I botched my "stg mail" so you weren't
included the first time]

Makes sense, ACK.

Post by Bjorn Helgaas

Post by Bjorn Helgaas
There's only one audit_classify_compat_syscall() definition, so it doesn't
need to be weak.
Remove the "weak" attribute from the audit_classify_compat_syscall()
declaration.
---
include/linux/audit.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 22cfddb75566..1b6098beb46b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -86,7 +86,7 @@ extern unsigned compat_dir_class[];
extern unsigned compat_chattr_class[];
extern unsigned compat_signal_class[];
-extern int __weak audit_classify_compat_syscall(int abi, unsigned syscall);
+extern int audit_classify_compat_syscall(int abi, unsigned syscall);
/* audit_names->type values */
#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */

- RGB

--
Richard Guy Briggs <***@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

[Bjorn Helgaas]

For the following interfaces:

get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()

there is only one implementation, so they do not need to be marked "weak".

Remove the "weak" attribute from their declarations.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: David Cohen <***@linux.intel.com>
CC: Kuppuswamy Sathyanarayanan <***@linux.intel.com>
CC: ***@kernel.org
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/


-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

[Bjorn Helgaas]

[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

[sathyanarayanan kuppuswamy]

Hi Bjorn,

Post by Bjorn Helgaas
[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h

Please remove this file and move the contents to asm/intel-mid.h.

Post by Bjorn Helgaas

Post by Bjorn Helgaas
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

[David Cohen]

Hi Bjorn and Sathya,

Post by Vineet Gupta
Hi Bjorn,

Post by Bjorn Helgaas
[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h

Please remove this file and move the contents to asm/intel-mid.h.

I partially agree :)

Historically, this file was created because we could not build all intel
mid variants at once. So we have to select only one during compilation
time, which was fixed already.

But we don't need to expose those functions outside intel-mid's
directory, which means asm/intel-mid.h isn't the best option IMHO.

If you want, I can send a small re-work instead: we get rid of this
header file completely and simplify a bit what is exposed by
asm/intel-mid.h. Or you can keep this patch and then I send the re-work
on top of it. Anyway I'm fine.

Br, David

Post by Vineet Gupta

Post by Bjorn Helgaas

Post by Bjorn Helgaas
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

--
Sathyanarayanan Kuppuswamy
Android kernel developer

[Bjorn Helgaas]

On Thu, Oct 16, 2014 at 7:41 PM, David Cohen

Post by David Cohen
Hi Bjorn and Sathya,

Post by Vineet Gupta
Hi Bjorn,

Post by Bjorn Helgaas
[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h

Please remove this file and move the contents to asm/intel-mid.h.

I don't know or care enough about intel-mid to do this myself. Right
now, I'm just trying to remove unnecessary and incorrect usage of
"weak" in header files.

Post by David Cohen
I partially agree :)
Historically, this file was created because we could not build all intel
mid variants at once. So we have to select only one during compilation
time, which was fixed already.
But we don't need to expose those functions outside intel-mid's
directory, which means asm/intel-mid.h isn't the best option IMHO.
If you want, I can send a small re-work instead: we get rid of this
header file completely and simplify a bit what is exposed by
asm/intel-mid.h. Or you can keep this patch and then I send the re-work
on top of it. Anyway I'm fine.

It's fine with me if you want to rework this to remove the header
completely. When I pointed this out in January [1], you mentioned
plans for that. But I think we should merge this patch in the interim
to remove the use of "weak" in a header file. If we leave bad
examples in the tree, they just proliferate.

Post by David Cohen

Post by Vineet Gupta

Post by Bjorn Helgaas

Post by Bjorn Helgaas
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

--
Sathyanarayanan Kuppuswamy
Android kernel developer

[David Cohen]

Post by Bjorn Helgaas
On Thu, Oct 16, 2014 at 7:41 PM, David Cohen

Post by David Cohen
Hi Bjorn and Sathya,

Post by Vineet Gupta
Hi Bjorn,

Post by Bjorn Helgaas
[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h

Please remove this file and move the contents to asm/intel-mid.h.

I don't know or care enough about intel-mid to do this myself. Right
now, I'm just trying to remove unnecessary and incorrect usage of
"weak" in header files.

Post by David Cohen
I partially agree :)
Historically, this file was created because we could not build all intel
mid variants at once. So we have to select only one during compilation
time, which was fixed already.
But we don't need to expose those functions outside intel-mid's
directory, which means asm/intel-mid.h isn't the best option IMHO.
If you want, I can send a small re-work instead: we get rid of this
header file completely and simplify a bit what is exposed by
asm/intel-mid.h. Or you can keep this patch and then I send the re-work
on top of it. Anyway I'm fine.

It's fine with me if you want to rework this to remove the header
completely. When I pointed this out in January [1], you mentioned
plans for that. But I think we should merge this patch in the interim
to remove the use of "weak" in a header file. If we leave bad
examples in the tree, they just proliferate.

Yeah, a lot late :(
Please, go ahead with this patch and I send something on top of it.

BR, David

Post by Bjorn Helgaas

Post by David Cohen

Post by Vineet Gupta

Post by Bjorn Helgaas

Post by Bjorn Helgaas
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

--
Sathyanarayanan Kuppuswamy
Android kernel developer

[Ingo Molnar]

Post by Bjorn Helgaas
get_penwell_ops()
get_cloverview_ops()
get_tangier_ops()
there is only one implementation, so they do not need to be marked "weak".
Remove the "weak" attribute from their declarations.
---
arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 46aa25c8ce06..3c1c3866d82b 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -10,10 +10,9 @@
*/
-/* __attribute__((weak)) makes these declarations overridable */
/* For every CPU addition a new get_<cpuname>_ops interface needs
* to be added.
*/
-extern void *get_penwell_ops(void) __attribute__((weak));
-extern void *get_cloverview_ops(void) __attribute__((weak));
-extern void *get_tangier_ops(void) __attribute__((weak));
+extern void *get_penwell_ops(void);
+extern void *get_cloverview_ops(void);
+extern void *get_tangier_ops(void);

Acked-by: Ingo Molnar <***@kernel.org>

Thanks,

Ingo

[Bjorn Helgaas]

There's only one implementation of mips_cpc_phys_base(), and it's only used
within the same file, so it doesn't need to be weak, and it doesn't need an
extern declaration.

Remove the extern mips_cpc_phys_base() declaration and make it static.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: linux-***@linux-mips.org
---
arch/mips/include/asm/mips-cpc.h | 10 ----------
arch/mips/kernel/mips-cpc.c | 2 +-
2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/mips/include/asm/mips-cpc.h b/arch/mips/include/asm/mips-cpc.h
index e139a534e0fd..8ff92cd74bde 100644
--- a/arch/mips/include/asm/mips-cpc.h
+++ b/arch/mips/include/asm/mips-cpc.h
@@ -28,16 +28,6 @@ extern void __iomem *mips_cpc_base;
extern phys_t mips_cpc_default_phys_base(void);

/**
- * mips_cpc_phys_base - retrieve the physical base address of the CPC
- *
- * This function returns the physical base address of the Cluster Power
- * Controller memory mapped registers, or 0 if no Cluster Power Controller
- * is present. It may be overriden by individual platforms which determine
- * this address in a different way.
- */
-extern phys_t __weak mips_cpc_phys_base(void);
-
-/**
* mips_cpc_probe - probe for a Cluster Power Controller
*
* Attempt to detect the presence of a Cluster Power Controller. Returns 0 if
diff --git a/arch/mips/kernel/mips-cpc.c b/arch/mips/kernel/mips-cpc.c
index ba473608a347..36c20ae509d8 100644
--- a/arch/mips/kernel/mips-cpc.c
+++ b/arch/mips/kernel/mips-cpc.c
@@ -21,7 +21,7 @@ static DEFINE_PER_CPU_ALIGNED(spinlock_t, cpc_core_lock);

static DEFINE_PER_CPU_ALIGNED(unsigned long, cpc_core_lock_flags);

-phys_t __weak mips_cpc_phys_base(void)
+static phys_t mips_cpc_phys_base(void)
{
u32 cpc_base;

[Bjorn Helgaas]

Post by Bjorn Helgaas
There's only one implementation of mips_cpc_phys_base(), and it's only used
within the same file, so it doesn't need to be weak, and it doesn't need an
extern declaration.
Remove the extern mips_cpc_phys_base() declaration and make it static.
---
arch/mips/include/asm/mips-cpc.h | 10 ----------
arch/mips/kernel/mips-cpc.c | 2 +-
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/mips/include/asm/mips-cpc.h b/arch/mips/include/asm/mips-cpc.h
index e139a534e0fd..8ff92cd74bde 100644
--- a/arch/mips/include/asm/mips-cpc.h
+++ b/arch/mips/include/asm/mips-cpc.h
@@ -28,16 +28,6 @@ extern void __iomem *mips_cpc_base;
extern phys_t mips_cpc_default_phys_base(void);
/**
- * mips_cpc_phys_base - retrieve the physical base address of the CPC
- *
- * This function returns the physical base address of the Cluster Power
- * Controller memory mapped registers, or 0 if no Cluster Power Controller
- * is present. It may be overriden by individual platforms which determine
- * this address in a different way.
- */
-extern phys_t __weak mips_cpc_phys_base(void);
-
-/**
* mips_cpc_probe - probe for a Cluster Power Controller
*
* Attempt to detect the presence of a Cluster Power Controller. Returns 0 if
diff --git a/arch/mips/kernel/mips-cpc.c b/arch/mips/kernel/mips-cpc.c
index ba473608a347..36c20ae509d8 100644
--- a/arch/mips/kernel/mips-cpc.c
+++ b/arch/mips/kernel/mips-cpc.c
@@ -21,7 +21,7 @@ static DEFINE_PER_CPU_ALIGNED(spinlock_t, cpc_core_lock);
static DEFINE_PER_CPU_ALIGNED(unsigned long, cpc_core_lock_flags);
-phys_t __weak mips_cpc_phys_base(void)
+static phys_t mips_cpc_phys_base(void)
{
u32 cpc_base;

[Bjorn Helgaas]

[+cc linux-mips for real this time. sheesh]

Post by Bjorn Helgaas
There's only one implementation of mips_cpc_phys_base(), and it's only used
within the same file, so it doesn't need to be weak, and it doesn't need an
extern declaration.
Remove the extern mips_cpc_phys_base() declaration and make it static.
---
arch/mips/include/asm/mips-cpc.h | 10 ----------
arch/mips/kernel/mips-cpc.c | 2 +-
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/mips/include/asm/mips-cpc.h b/arch/mips/include/asm/mips-cpc.h
index e139a534e0fd..8ff92cd74bde 100644
--- a/arch/mips/include/asm/mips-cpc.h
+++ b/arch/mips/include/asm/mips-cpc.h
@@ -28,16 +28,6 @@ extern void __iomem *mips_cpc_base;
extern phys_t mips_cpc_default_phys_base(void);
/**
- * mips_cpc_phys_base - retrieve the physical base address of the CPC
- *
- * This function returns the physical base address of the Cluster Power
- * Controller memory mapped registers, or 0 if no Cluster Power Controller
- * is present. It may be overriden by individual platforms which determine
- * this address in a different way.
- */
-extern phys_t __weak mips_cpc_phys_base(void);
-
-/**
* mips_cpc_probe - probe for a Cluster Power Controller
*
* Attempt to detect the presence of a Cluster Power Controller. Returns 0 if
diff --git a/arch/mips/kernel/mips-cpc.c b/arch/mips/kernel/mips-cpc.c
index ba473608a347..36c20ae509d8 100644
--- a/arch/mips/kernel/mips-cpc.c
+++ b/arch/mips/kernel/mips-cpc.c
@@ -21,7 +21,7 @@ static DEFINE_PER_CPU_ALIGNED(spinlock_t, cpc_core_lock);
static DEFINE_PER_CPU_ALIGNED(unsigned long, cpc_core_lock_flags);
-phys_t __weak mips_cpc_phys_base(void)
+static phys_t mips_cpc_phys_base(void)
{
u32 cpc_base;

[Bjorn Helgaas]

arch/mips/mm/init.c provides a default platform_maar_init() definition
explicitly marked "weak". arch/mips/mti-malta/malta-memory.c provides its
own definition intended to override the default, but the "weak" attribute
on the declaration applied to this as well, so the linker chose one based
on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).

Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: linux-***@linux-mips.org
---
arch/mips/include/asm/maar.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
index 6c62b0f899c0..b02891f9caaf 100644
--- a/arch/mips/include/asm/maar.h
+++ b/arch/mips/include/asm/maar.h
@@ -26,7 +26,7 @@
*
* Return: The number of MAAR pairs configured.
*/
-unsigned __weak platform_maar_init(unsigned num_pairs);
+unsigned platform_maar_init(unsigned num_pairs);

/**
* write_maar_pair() - write to a pair of MAARs

[Bjorn Helgaas]

[+cc linux-mips]

Post by Bjorn Helgaas
arch/mips/mm/init.c provides a default platform_maar_init() definition
explicitly marked "weak". arch/mips/mti-malta/malta-memory.c provides its
own definition intended to override the default, but the "weak" attribute
on the declaration applied to this as well, so the linker chose one based
on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.
---
arch/mips/include/asm/maar.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
index 6c62b0f899c0..b02891f9caaf 100644
--- a/arch/mips/include/asm/maar.h
+++ b/arch/mips/include/asm/maar.h
@@ -26,7 +26,7 @@
*
* Return: The number of MAAR pairs configured.
*/
-unsigned __weak platform_maar_init(unsigned num_pairs);
+unsigned platform_maar_init(unsigned num_pairs);
/**
* write_maar_pair() - write to a pair of MAARs

[Bjorn Helgaas]

kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).

Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.

Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Daniel Lezcano <***@linaro.org>
CC: Martin Schwidefsky <***@de.ibm.com>
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);

extern u64

[John Stultz]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")

Acked-by: John Stultz <***@linaro.org>

[Bjorn Helgaas]

[+cc Daniel, Martin, linux-s390; sorry, I botched my "stg mail" so you
weren't included the first time. s390 could see issues from this.]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);
extern u64

[Martin Schwidefsky]

On Wed, 15 Oct 2014 17:30:33 -0600

Post by Bjorn Helgaas
[+cc Daniel, Martin, linux-s390; sorry, I botched my "stg mail" so you
weren't included the first time. s390 could see issues from this.]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);
extern u64

s390 compiles and boots without the __weak for clocksource_default_clock.

[Bjorn Helgaas]

On Thu, Oct 16, 2014 at 1:22 AM, Martin Schwidefsky

Post by Martin Schwidefsky
On Wed, 15 Oct 2014 17:30:33 -0600

Post by Bjorn Helgaas
[+cc Daniel, Martin, linux-s390; sorry, I botched my "stg mail" so you
weren't included the first time. s390 could see issues from this.]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);
extern u64

s390 compiles and boots without the __weak for clocksource_default_clock.

I assume this means you've tested this patch and s390 compiles and boots?

I assume you *don't* mean that s390 could drop its
clocksource_default_clock() implementation and use the generic one,
right?

Bjorn

[Martin Schwidefsky]

On Thu, 16 Oct 2014 07:40:16 -0600

Post by Bjorn Helgaas
On Thu, Oct 16, 2014 at 1:22 AM, Martin Schwidefsky

Post by Martin Schwidefsky
On Wed, 15 Oct 2014 17:30:33 -0600

Post by Bjorn Helgaas
[+cc Daniel, Martin, linux-s390; sorry, I botched my "stg mail" so you
weren't included the first time. s390 could see issues from this.]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);
extern u64

s390 compiles and boots without the __weak for clocksource_default_clock.

I assume this means you've tested this patch and s390 compiles and boots?

Correct.

Post by Bjorn Helgaas
I assume you *don't* mean that s390 could drop its
clocksource_default_clock() implementation and use the generic one,
right?

Sorry, but I want to keep the s390 TOD clock as default.
The jiffies clock is not precise enough, even if it is used only at the beginning.

[Ingo Molnar]

Post by Bjorn Helgaas
kernel/time/jiffies.c provides a default clocksource_default_clock()
definition explicitly marked "weak". arch/s390 provides its own definition
intended to override the default, but the "weak" attribute on the
declaration applied to the s390 definition as well, so the linker chose one
based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the clocksource_default_clock()
declaration so we always prefer a non-weak definition over the weak one,
independent of link order.
Fixes: f1b82746c1e9 ("clocksource: Cleanup clocksource selection")
---
include/linux/clocksource.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 653f0e2b6ca9..abcafaa20b86 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -287,7 +287,7 @@ extern struct clocksource* clocksource_get_next(void);
extern void clocksource_change_rating(struct clocksource *cs, int rating);
extern void clocksource_suspend(void);
extern void clocksource_resume(void);
-extern struct clocksource * __init __weak clocksource_default_clock(void);
+extern struct clocksource * __init clocksource_default_clock(void);
extern void clocksource_mark_unstable(struct clocksource *cs);

Acked-by: Ingo Molnar <***@kernel.org>

Thanks,

Ingo

[Bjorn Helgaas]

When the "weak" attribute is on a declaration in a header file, every
definition where the header is included becomes weak, and the linker
chooses one definition based on link order (see 10629d711ed7 ("PCI: Remove
__weak annotation from pcibios_get_phb_of_node decl")).

Move the "weak" attribute from the declaration to the default definition so
we always prefer a non-weak definition over the weak one, independent of
link order.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: linux-***@linux-mips.org
---
arch/mips/include/asm/vpe.h | 2 +-
arch/mips/kernel/vpe-mt.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/vpe.h b/arch/mips/include/asm/vpe.h
index 7849f3978fea..80e70dbd1f64 100644
--- a/arch/mips/include/asm/vpe.h
+++ b/arch/mips/include/asm/vpe.h
@@ -122,7 +122,7 @@ void release_vpe(struct vpe *v);
void *alloc_progmem(unsigned long len);
void release_progmem(void *ptr);

-int __weak vpe_run(struct vpe *v);
+int vpe_run(struct vpe *v);
void cleanup_tc(struct tc *tc);

int __init vpe_module_init(void);
diff --git a/arch/mips/kernel/vpe-mt.c b/arch/mips/kernel/vpe-mt.c
index 2e003b11a098..0e5899a2cd96 100644
--- a/arch/mips/kernel/vpe-mt.c
+++ b/arch/mips/kernel/vpe-mt.c
@@ -23,7 +23,7 @@ static int major;
static int hw_tcs, hw_vpes;

/* We are prepared so configure and start the VPE... */
-int vpe_run(struct vpe *v)
+int __weak vpe_run(struct vpe *v)
{
unsigned long flags, val, dmt_flag;
struct vpe_notifications *notifier;

[Bjorn Helgaas]

[+cc linux-mips]

Post by Bjorn Helgaas
When the "weak" attribute is on a declaration in a header file, every
definition where the header is included becomes weak, and the linker
chooses one definition based on link order (see 10629d711ed7 ("PCI: Remove
__weak annotation from pcibios_get_phb_of_node decl")).
Move the "weak" attribute from the declaration to the default definition so
we always prefer a non-weak definition over the weak one, independent of
link order.
---
arch/mips/include/asm/vpe.h | 2 +-
arch/mips/kernel/vpe-mt.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/include/asm/vpe.h b/arch/mips/include/asm/vpe.h
index 7849f3978fea..80e70dbd1f64 100644
--- a/arch/mips/include/asm/vpe.h
+++ b/arch/mips/include/asm/vpe.h
@@ -122,7 +122,7 @@ void release_vpe(struct vpe *v);
void *alloc_progmem(unsigned long len);
void release_progmem(void *ptr);
-int __weak vpe_run(struct vpe *v);
+int vpe_run(struct vpe *v);
void cleanup_tc(struct tc *tc);
int __init vpe_module_init(void);
diff --git a/arch/mips/kernel/vpe-mt.c b/arch/mips/kernel/vpe-mt.c
index 2e003b11a098..0e5899a2cd96 100644
--- a/arch/mips/kernel/vpe-mt.c
+++ b/arch/mips/kernel/vpe-mt.c
@@ -23,7 +23,7 @@ static int major;
static int hw_tcs, hw_vpes;
/* We are prepared so configure and start the VPE... */
-int vpe_run(struct vpe *v)
+int __weak vpe_run(struct vpe *v)
{
unsigned long flags, val, dmt_flag;
struct vpe_notifications *notifier;

[Bjorn Helgaas]

[+cc Stephen]

Post by Bjorn Helgaas
[+cc linux-mips]

Post by Bjorn Helgaas
When the "weak" attribute is on a declaration in a header file, every
definition where the header is included becomes weak, and the linker
chooses one definition based on link order (see 10629d711ed7 ("PCI: Remove
__weak annotation from pcibios_get_phb_of_node decl")).
Move the "weak" attribute from the declaration to the default definition so
we always prefer a non-weak definition over the weak one, independent of
link order.
---
arch/mips/include/asm/vpe.h | 2 +-
arch/mips/kernel/vpe-mt.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/include/asm/vpe.h b/arch/mips/include/asm/vpe.h
index 7849f3978fea..80e70dbd1f64 100644
--- a/arch/mips/include/asm/vpe.h
+++ b/arch/mips/include/asm/vpe.h
@@ -122,7 +122,7 @@ void release_vpe(struct vpe *v);
void *alloc_progmem(unsigned long len);
void release_progmem(void *ptr);
-int __weak vpe_run(struct vpe *v);
+int vpe_run(struct vpe *v);
void cleanup_tc(struct tc *tc);
int __init vpe_module_init(void);
diff --git a/arch/mips/kernel/vpe-mt.c b/arch/mips/kernel/vpe-mt.c
index 2e003b11a098..0e5899a2cd96 100644
--- a/arch/mips/kernel/vpe-mt.c
+++ b/arch/mips/kernel/vpe-mt.c
@@ -23,7 +23,7 @@ static int major;
static int hw_tcs, hw_vpes;
/* We are prepared so configure and start the VPE... */
-int vpe_run(struct vpe *v)
+int __weak vpe_run(struct vpe *v)
{
unsigned long flags, val, dmt_flag;
struct vpe_notifications *notifier;

Just FYI, this patch was in linux-next today, but I dropped it
temporarily because Fengguang's auto-builder found the following issue

Post by Bjorn Helgaas

Post by Bjorn Helgaas
arch/mips/kernel/vpe.c:830:29: error: the address of 'vpe_run' will always evaluate as 'true' [-Werror=address]

if ((vpe_elfload(v) >= 0) && vpe_run) {
^
cc1: all warnings being treated as errors

[Bjorn Helgaas]

For the following functions:

elfcorehdr_alloc()
elfcorehdr_free()
elfcorehdr_read()
elfcorehdr_read_notes()
remap_oldmem_pfn_range()

fs/proc/vmcore.c provides default definitions explicitly marked "weak".
arch/s390 provides its own definitions intended to override the default
ones, but the "weak" attribute on the declarations applied to the s390
definitions as well, so the linker chose one based on link order (see
10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
decl")).

Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.

Fixes: be8a8d069e50 ("vmcore: introduce ELF header in new memory feature")
Fixes: 9cb218131de1 ("vmcore: introduce remap_oldmem_pfn_range()")
Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Michael Holzheu <***@linux.vnet.ibm.com>
CC: Vivek Goyal <***@redhat.com>
---
include/linux/crash_dump.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 72ab536ad3de..3849fce7ecfe 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,14 +14,13 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;

-extern int __weak elfcorehdr_alloc(unsigned long long *addr,
- unsigned long long *size);
-extern void __weak elfcorehdr_free(unsigned long long addr);
-extern ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos);
-extern ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
-extern int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma,
- unsigned long from, unsigned long pfn,
- unsigned long size, pgprot_t prot);
+extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
+extern void elfcorehdr_free(unsigned long long addr);
+extern ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos);
+extern ssize_t elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
+extern int remap_oldmem_pfn_range(struct vm_area_struct *vma,
+ unsigned long from, unsigned long pfn,
+ unsigned long size, pgprot_t prot);

extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);

[Bjorn Helgaas]

[+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
included the first time]

Post by Bjorn Helgaas
elfcorehdr_alloc()
elfcorehdr_free()
elfcorehdr_read()
elfcorehdr_read_notes()
remap_oldmem_pfn_range()
fs/proc/vmcore.c provides default definitions explicitly marked "weak".
arch/s390 provides its own definitions intended to override the default
ones, but the "weak" attribute on the declarations applied to the s390
definitions as well, so the linker chose one based on link order (see
10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: be8a8d069e50 ("vmcore: introduce ELF header in new memory feature")
Fixes: 9cb218131de1 ("vmcore: introduce remap_oldmem_pfn_range()")
---
include/linux/crash_dump.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 72ab536ad3de..3849fce7ecfe 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,14 +14,13 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;
-extern int __weak elfcorehdr_alloc(unsigned long long *addr,
- unsigned long long *size);
-extern void __weak elfcorehdr_free(unsigned long long addr);
-extern ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos);
-extern ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
-extern int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma,
- unsigned long from, unsigned long pfn,
- unsigned long size, pgprot_t prot);
+extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
+extern void elfcorehdr_free(unsigned long long addr);
+extern ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos);
+extern ssize_t elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
+extern int remap_oldmem_pfn_range(struct vm_area_struct *vma,
+ unsigned long from, unsigned long pfn,
+ unsigned long size, pgprot_t prot);
extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);

[Bjorn Helgaas]

[+cc Martin, Heiko, linux-s390, since s390 could see issues from this.
Sorry for all the spam; I should have gotten this right the first
time.]

Post by Bjorn Helgaas
[+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
included the first time]

Post by Bjorn Helgaas
elfcorehdr_alloc()
elfcorehdr_free()
elfcorehdr_read()
elfcorehdr_read_notes()
remap_oldmem_pfn_range()
fs/proc/vmcore.c provides default definitions explicitly marked "weak".
arch/s390 provides its own definitions intended to override the default
ones, but the "weak" attribute on the declarations applied to the s390
definitions as well, so the linker chose one based on link order (see
10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: be8a8d069e50 ("vmcore: introduce ELF header in new memory feature")
Fixes: 9cb218131de1 ("vmcore: introduce remap_oldmem_pfn_range()")
---
include/linux/crash_dump.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 72ab536ad3de..3849fce7ecfe 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,14 +14,13 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;
-extern int __weak elfcorehdr_alloc(unsigned long long *addr,
- unsigned long long *size);
-extern void __weak elfcorehdr_free(unsigned long long addr);
-extern ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos);
-extern ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
-extern int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma,
- unsigned long from, unsigned long pfn,
- unsigned long size, pgprot_t prot);
+extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
+extern void elfcorehdr_free(unsigned long long addr);
+extern ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos);
+extern ssize_t elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
+extern int remap_oldmem_pfn_range(struct vm_area_struct *vma,
+ unsigned long from, unsigned long pfn,
+ unsigned long size, pgprot_t prot);
extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);

[Martin Schwidefsky]

On Wed, 15 Oct 2014 17:44:29 -0600

Post by Bjorn Helgaas
[+cc Martin, Heiko, linux-s390, since s390 could see issues from this.
Sorry for all the spam; I should have gotten this right the first
time.]

Post by Bjorn Helgaas
[+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
included the first time]

Post by Bjorn Helgaas
elfcorehdr_alloc()
elfcorehdr_free()
elfcorehdr_read()
elfcorehdr_read_notes()
remap_oldmem_pfn_range()
fs/proc/vmcore.c provides default definitions explicitly marked "weak".
arch/s390 provides its own definitions intended to override the default
ones, but the "weak" attribute on the declarations applied to the s390
definitions as well, so the linker chose one based on link order (see
10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: be8a8d069e50 ("vmcore: introduce ELF header in new memory feature")
Fixes: 9cb218131de1 ("vmcore: introduce remap_oldmem_pfn_range()")
---
include/linux/crash_dump.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 72ab536ad3de..3849fce7ecfe 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,14 +14,13 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;
-extern int __weak elfcorehdr_alloc(unsigned long long *addr,
- unsigned long long *size);
-extern void __weak elfcorehdr_free(unsigned long long addr);
-extern ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos);
-extern ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
-extern int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma,
- unsigned long from, unsigned long pfn,
- unsigned long size, pgprot_t prot);
+extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
+extern void elfcorehdr_free(unsigned long long addr);
+extern ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos);
+extern ssize_t elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
+extern int remap_oldmem_pfn_range(struct vm_area_struct *vma,
+ unsigned long from, unsigned long pfn,
+ unsigned long size, pgprot_t prot);
extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);

These __weak attributes can be removed as well without ill effect for s390.

[Vivek Goyal]

Post by Bjorn Helgaas
[+cc Martin, Heiko, linux-s390, since s390 could see issues from this.
Sorry for all the spam; I should have gotten this right the first
time.]

Post by Bjorn Helgaas
[+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
included the first time]

Post by Bjorn Helgaas
elfcorehdr_alloc()
elfcorehdr_free()
elfcorehdr_read()
elfcorehdr_read_notes()
remap_oldmem_pfn_range()
fs/proc/vmcore.c provides default definitions explicitly marked "weak".
arch/s390 provides its own definitions intended to override the default
ones, but the "weak" attribute on the declarations applied to the s390
definitions as well, so the linker chose one based on link order (see
10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node
decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: be8a8d069e50 ("vmcore: introduce ELF header in new memory feature")
Fixes: 9cb218131de1 ("vmcore: introduce remap_oldmem_pfn_range()")

Looks good to me.

Acked-by: Vivek Goyal <***@redhat.com>

Thanks
Vivek

Post by Bjorn Helgaas

Post by Bjorn Helgaas

Post by Bjorn Helgaas
---
include/linux/crash_dump.h | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 72ab536ad3de..3849fce7ecfe 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -14,14 +14,13 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;
-extern int __weak elfcorehdr_alloc(unsigned long long *addr,
- unsigned long long *size);
-extern void __weak elfcorehdr_free(unsigned long long addr);
-extern ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos);
-extern ssize_t __weak elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
-extern int __weak remap_oldmem_pfn_range(struct vm_area_struct *vma,
- unsigned long from, unsigned long pfn,
- unsigned long size, pgprot_t prot);
+extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
+extern void elfcorehdr_free(unsigned long long addr);
+extern ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos);
+extern ssize_t elfcorehdr_read_notes(char *buf, size_t count, u64 *ppos);
+extern int remap_oldmem_pfn_range(struct vm_area_struct *vma,
+ unsigned long from, unsigned long pfn,
+ unsigned long size, pgprot_t prot);
extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
unsigned long, int);

[Bjorn Helgaas]

kernel/debug/debug_core.c provides a default kgdb_arch_pc() definition
explicitly marked "weak". Several architectures provide their own
definitions intended to override the default, but the "weak" attribute on
the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).

Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.

Fixes: 688b744d8bc8 ("kgdb: fix signedness mixmatches, add statics, add declaration to header")
Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Harvey Harrison <***@gmail.com>
---
include/linux/kgdb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 6b06d378f3df..e465bb15912d 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -283,7 +283,7 @@ struct kgdb_io {

extern struct kgdb_arch arch_kgdb_ops;

-extern unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs);
+extern unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs);

#ifdef CONFIG_SERIAL_KGDB_NMI
extern int kgdb_register_nmi_console(void);

[Bjorn Helgaas]

[+cc Harvey, Vineet, linux-sh; arc, sh, and x86 have kgdb_arch_pc()
definitions and could see issues from this]

Post by Bjorn Helgaas
kernel/debug/debug_core.c provides a default kgdb_arch_pc() definition
explicitly marked "weak". Several architectures provide their own
definitions intended to override the default, but the "weak" attribute on
the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: 688b744d8bc8 ("kgdb: fix signedness mixmatches, add statics, add declaration to header")
---
include/linux/kgdb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 6b06d378f3df..e465bb15912d 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -283,7 +283,7 @@ struct kgdb_io {
extern struct kgdb_arch arch_kgdb_ops;
-extern unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs);
+extern unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs);
#ifdef CONFIG_SERIAL_KGDB_NMI
extern int kgdb_register_nmi_console(void);

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Vineet Gupta]

This post might be inappropriate. Click to display it.

[Vineet Gupta]

Post by Vineet Gupta

Post by Bjorn Helgaas
[+cc Harvey, Vineet, linux-sh; arc, sh, and x86 have kgdb_arch_pc()
definitions and could see issues from this]

Thanks for the heads up !
I compile tested arc tree (with kgdb) from linux-next of today, which has this
patch already and things seem to be ok.
As a hack I reverted the patch and see that kgdb_arch_pc() becomes weak, although
ARC version was still being pulled in final - it was recored as weak and that
cause bloat-o-meter to go awry.
bloat-o-meter vmlinux-with-fix vmlinux-before-fix
add/remove: 0/1 grow/shrink: 1/0 up/down: 1/-8 (-7)
function old new delta
vermagic 36 37 +1
kgdb_arch_pc 8 - -8

The ARC definition of kgdb_arch_pc is same as generic one so we might as well drop it.
Can you add the patch below to your series. If not I can add it to ARC changes for
this merge window. Please let me know.

Thx,
-Vineet

---------------------->
From f01b1db6986924e794eddc038167329d63d1fda9 Mon Sep 17 00:00:00 2001
From: Vineet Gupta <***@synopsys.com>
Date: Fri, 17 Oct 2014 13:27:03 +0530
Subject: [PATCH] ARC: kgdb: generic kgdb_arch_pc() suffices

Signed-off-by: Vineet Gupta <***@synopsys.com>
---
arch/arc/kernel/kgdb.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
index a2ff5c5d1450..ecf6a7869375 100644
--- a/arch/arc/kernel/kgdb.c
+++ b/arch/arc/kernel/kgdb.c
@@ -158,11 +158,6 @@ int kgdb_arch_handle_exception(int e_vector, int signo, int
err_code,
return -1;
}

-unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs)
-{
- return instruction_pointer(regs);
-}
-
int kgdb_arch_init(void)
{
single_step_data.armed = 0;

[Bjorn Helgaas]

On Fri, Oct 17, 2014 at 1:59 AM, Vineet Gupta

Post by Vineet Gupta

Post by Vineet Gupta

Post by Bjorn Helgaas
[+cc Harvey, Vineet, linux-sh; arc, sh, and x86 have kgdb_arch_pc()
definitions and could see issues from this]

Thanks for the heads up !
I compile tested arc tree (with kgdb) from linux-next of today, which has this
patch already and things seem to be ok.
As a hack I reverted the patch and see that kgdb_arch_pc() becomes weak, although
ARC version was still being pulled in final - it was recored as weak and that
cause bloat-o-meter to go awry.
bloat-o-meter vmlinux-with-fix vmlinux-before-fix
add/remove: 0/1 grow/shrink: 1/0 up/down: 1/-8 (-7)
function old new delta
vermagic 36 37 +1
kgdb_arch_pc 8 - -8

The ARC definition of kgdb_arch_pc is same as generic one so we might as well drop it.
Can you add the patch below to your series. If not I can add it to ARC changes for
this merge window. Please let me know.

I added the patch below to my series. The merge window just closed,
but I'll try to merge these as fixes before v3.18. Thanks!

Bjorn

Post by Vineet Gupta
---------------------->
From f01b1db6986924e794eddc038167329d63d1fda9 Mon Sep 17 00:00:00 2001
Date: Fri, 17 Oct 2014 13:27:03 +0530
Subject: [PATCH] ARC: kgdb: generic kgdb_arch_pc() suffices
---
arch/arc/kernel/kgdb.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
index a2ff5c5d1450..ecf6a7869375 100644
--- a/arch/arc/kernel/kgdb.c
+++ b/arch/arc/kernel/kgdb.c
@@ -158,11 +158,6 @@ int kgdb_arch_handle_exception(int e_vector, int signo, int
err_code,
return -1;
}
-unsigned long kgdb_arch_pc(int exception, struct pt_regs *regs)
-{
- return instruction_pointer(regs);
-}
-
int kgdb_arch_init(void)
{
single_step_data.armed = 0;
--
1.8.3.2

[Harvey Harrison]

Post by Bjorn Helgaas
kernel/debug/debug_core.c provides a default kgdb_arch_pc() definition
explicitly marked "weak". Several architectures provide their own
definitions intended to override the default, but the "weak" attribute on
the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: 688b744d8bc8 ("kgdb: fix signedness mixmatches, add statics, add declaration to header")

Reviewed-by: Harvey Harrison <***@gmail.com>

This was likely simply an error in my patch, likely just copied the
function definition and didn't even notice the
weak annotation at the time.

Harvey

[Bjorn Helgaas]

drivers/base/memory.c provides a default memory_block_size_bytes()
definition explicitly marked "weak". Several architectures provide their
own definitions intended to override the default, but the "weak" attribute
on the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).

Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.

Fixes: 41f107266b19 ("drivers: base: Add prototype declaration to the header file")
Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Rashika Kheria <***@gmail.com>
CC: Nathan Fontenot <***@austin.ibm.com>
CC: Anton Blanchard <***@au1.ibm.com>
CC: Heiko Carstens <***@de.ibm.com>
CC: Yinghai Lu <***@kernel.org>
---
include/linux/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/memory.h b/include/linux/memory.h
index bb7384e3c3d8..8b8d8d12348e 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -35,7 +35,7 @@ struct memory_block {
};

int arch_get_memory_phys_device(unsigned long start_pfn);
-unsigned long __weak memory_block_size_bytes(void);
+unsigned long memory_block_size_bytes(void);

/* These states are exposed to userspace as text strings in sysfs */
#define MEM_ONLINE (1<<0) /* exposed to userspace */

[Bjorn Helgaas]

[+cc Rashika, Nathan, Anton, Blanchard, Heiko, Yinghai, Martin,
linux-s390; sorry, I botched my "stg mail" so you weren't included the
first time. s390 and x86 define their own memory_block_size_bytes()
and are at risk for this problem.]

Post by Bjorn Helgaas
drivers/base/memory.c provides a default memory_block_size_bytes()
definition explicitly marked "weak". Several architectures provide their
own definitions intended to override the default, but the "weak" attribute
on the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: 41f107266b19 ("drivers: base: Add prototype declaration to the header file")
---
include/linux/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/memory.h b/include/linux/memory.h
index bb7384e3c3d8..8b8d8d12348e 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -35,7 +35,7 @@ struct memory_block {
};
int arch_get_memory_phys_device(unsigned long start_pfn);
-unsigned long __weak memory_block_size_bytes(void);
+unsigned long memory_block_size_bytes(void);
/* These states are exposed to userspace as text strings in sysfs */
#define MEM_ONLINE (1<<0) /* exposed to userspace */

[Martin Schwidefsky]

On Wed, 15 Oct 2014 17:38:02 -0600

Post by Bjorn Helgaas
[+cc Rashika, Nathan, Anton, Blanchard, Heiko, Yinghai, Martin,
linux-s390; sorry, I botched my "stg mail" so you weren't included the
first time. s390 and x86 define their own memory_block_size_bytes()
and are at risk for this problem.]

Post by Bjorn Helgaas
drivers/base/memory.c provides a default memory_block_size_bytes()
definition explicitly marked "weak". Several architectures provide their
own definitions intended to override the default, but the "weak" attribute
on the declaration applied to the arch definitions as well, so the linker
chose one based on link order (see 10629d711ed7 ("PCI: Remove __weak
annotation from pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declaration so we always prefer a
non-weak definition over the weak one, independent of link order.
Fixes: 41f107266b19 ("drivers: base: Add prototype declaration to the header file")
---
include/linux/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/memory.h b/include/linux/memory.h
index bb7384e3c3d8..8b8d8d12348e 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -35,7 +35,7 @@ struct memory_block {
};
int arch_get_memory_phys_device(unsigned long start_pfn);
-unsigned long __weak memory_block_size_bytes(void);
+unsigned long memory_block_size_bytes(void);
/* These states are exposed to userspace as text strings in sysfs */
#define MEM_ONLINE (1<<0) /* exposed to userspace */

s390 works fine with the __weak on memory_bloc_size_bytes.

[Bjorn Helgaas]

For the following interfaces:

set_swbp()
set_orig_insn()
is_swbp_insn()
is_trap_insn()
uprobe_get_swbp_addr()
arch_uprobe_ignore()
arch_uprobe_copy_ixol()

kernel/events/uprobes.c provides default definitions explicitly marked
"weak". Some architectures provide their own definitions intended to
override the defaults, but the "weak" attribute on the declarations applied
to the arch definitions as well, so the linker chose one based on link
order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).

Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.

Signed-off-by: Bjorn Helgaas <***@google.com>
CC: Victor Kamensky <***@linaro.org>
CC: Oleg Nesterov <***@redhat.com>
CC: David A. Long <***@linaro.org>
CC: Srikar Dronamraju <***@linux.vnet.ibm.com>
CC: Ananth N Mavinakayanahalli <***@in.ibm.com>
---
include/linux/uprobes.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 4f844c6b03ee..60beb5dc7977 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -98,11 +98,11 @@ struct uprobes_state {
struct xol_area *xol_area;
};

-extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
-extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
-extern unsigned long __weak uprobe_get_swbp_addr(struct pt_regs *regs);
+extern int set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern int set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern bool is_swbp_insn(uprobe_opcode_t *insn);
+extern bool is_trap_insn(uprobe_opcode_t *insn);
+extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs);
extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs);
extern int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
@@ -128,8 +128,8 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
-extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
+extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
+extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len);
#else /* !CONFIG_UPROBES */
struct uprobes_state {

[Bjorn Helgaas]

[+cc Victor, Oleg, David, Srikar, Ananth, Russell, linux-arm-kernel,
Ben, Paul, Michael, linuxppc-dev. arm and powerpc define some of
these functions and are at risk for this issue.]

Post by Bjorn Helgaas
set_swbp()
set_orig_insn()
is_swbp_insn()
is_trap_insn()
uprobe_get_swbp_addr()
arch_uprobe_ignore()
arch_uprobe_copy_ixol()
kernel/events/uprobes.c provides default definitions explicitly marked
"weak". Some architectures provide their own definitions intended to
override the defaults, but the "weak" attribute on the declarations applied
to the arch definitions as well, so the linker chose one based on link
order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
---
include/linux/uprobes.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 4f844c6b03ee..60beb5dc7977 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -98,11 +98,11 @@ struct uprobes_state {
struct xol_area *xol_area;
};
-extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
-extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
-extern unsigned long __weak uprobe_get_swbp_addr(struct pt_regs *regs);
+extern int set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern int set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern bool is_swbp_insn(uprobe_opcode_t *insn);
+extern bool is_trap_insn(uprobe_opcode_t *insn);
+extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs);
extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs);
extern int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
@@ -128,8 +128,8 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
-extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
+extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
+extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len);
#else /* !CONFIG_UPROBES */
struct uprobes_state {

[Ingo Molnar]

Post by Bjorn Helgaas
set_swbp()
set_orig_insn()
is_swbp_insn()
is_trap_insn()
uprobe_get_swbp_addr()
arch_uprobe_ignore()
arch_uprobe_copy_ixol()
kernel/events/uprobes.c provides default definitions explicitly marked
"weak". Some architectures provide their own definitions intended to
override the defaults, but the "weak" attribute on the declarations applied
to the arch definitions as well, so the linker chose one based on link
order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
---
include/linux/uprobes.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 4f844c6b03ee..60beb5dc7977 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -98,11 +98,11 @@ struct uprobes_state {
struct xol_area *xol_area;
};
-extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
-extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
-extern unsigned long __weak uprobe_get_swbp_addr(struct pt_regs *regs);
+extern int set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern int set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern bool is_swbp_insn(uprobe_opcode_t *insn);
+extern bool is_trap_insn(uprobe_opcode_t *insn);
+extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs);
extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs);
extern int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
@@ -128,8 +128,8 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
-extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
+extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
+extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len);
#else /* !CONFIG_UPROBES */
struct uprobes_state {

Acked-by: Ingo Molnar <***@kernel.org>

Thanks,

Ingo

[Srikar Dronamraju]

Post by Bjorn Helgaas
set_swbp()
set_orig_insn()
is_swbp_insn()
is_trap_insn()
uprobe_get_swbp_addr()
arch_uprobe_ignore()
arch_uprobe_copy_ixol()
kernel/events/uprobes.c provides default definitions explicitly marked
"weak". Some architectures provide their own definitions intended to
override the defaults, but the "weak" attribute on the declarations applied
to the arch definitions as well, so the linker chose one based on link
order (see 10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")).
Remove the "weak" attribute from the declarations so we always prefer a
non-weak definition over the weak one, independent of link order.
---
include/linux/uprobes.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
index 4f844c6b03ee..60beb5dc7977 100644
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -98,11 +98,11 @@ struct uprobes_state {
struct xol_area *xol_area;
};
-extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
-extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
-extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
-extern unsigned long __weak uprobe_get_swbp_addr(struct pt_regs *regs);
+extern int set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern int set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
+extern bool is_swbp_insn(uprobe_opcode_t *insn);
+extern bool is_trap_insn(uprobe_opcode_t *insn);
+extern unsigned long uprobe_get_swbp_addr(struct pt_regs *regs);
extern unsigned long uprobe_get_trap_addr(struct pt_regs *regs);
extern int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr, uprobe_opcode_t);
extern int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *uc);
@@ -128,8 +128,8 @@ extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned long val, void *data);
extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs);
extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs);
-extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
-extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
+extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
+extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len);
#else /* !CONFIG_UPROBES */
struct uprobes_state {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Andrew Morton]

Post by Bjorn Helgaas
A common usage of "weak" is for a default implementation of a function.
An architecture that needs something different can supply a non-weak
("strong") implementation, with the expectation that the linker will select
the strong version and discard the weak default version.
We have a few function declarations in header files annotated as "weak".
That causes every *every* definition to be marked "weak", which means there
is no strong version at all. In this case, the linker selects one of the
weak versions based on link order. I don't think this is what we want.
These patches remove almost all the weak annotations from header files
(MIPS still uses it for get_c0_compare_int(), apparently relying on the
fact that a weak symbol need not be defined at all). In most cases, the
default implementation was already marked weak at the definition. When it
wasn't, I added that.
It might be simplest if I ask Linus to pull these all as a group from my
branch [1]. I'll look for acks from the following people. If I don't see
an ack, I'll drop the patch and you can take it yourself or ignore it as
you wish.
Eric: audit
Thomas, Ingo, or Peter: x86
Ralf: MIPS
John or Thomas: clocksource
Jason: kgdb
Ingo: uprobes
Andrew: vmcore, memory-hotplug

Acks, of course..

Post by Bjorn Helgaas
I don't know whether these fix any actual bugs. We *did* have a bug like
this on MIPS a while ago (10629d711ed7 ("PCI: Remove __weak annotation from
pcibios_get_phb_of_node decl")), so it's possible that they do fix
something.

I'm rather astonished that we haven't hit problems with this before
now.

This is pretty rude behaviour from the linker, really - grabbing the
first __weak function and using that is very likely to be the wrong
thing to do.

Still, this is a bit of a hand grenade and we should think up some way
of detecting/preventing recurrences.

I guess a checkpatch rule which warns about __weak and
__attribute__((weak)) in a header file would help. Is there anything
more robust we can do? Coccinelle, sparse, etc?