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(-)
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 <[email protected]>
CC: AKASHI Takahiro <[email protected]>
CC: Richard Guy Briggs <[email protected]>
---
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 */
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 <[email protected]>
CC: David Cohen <[email protected]>
CC: Kuppuswamy Sathyanarayanan <[email protected]>
CC: [email protected]
---
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);
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 <[email protected]>
CC: [email protected]
---
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;
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 <[email protected]>
CC: [email protected]
---
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
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 <[email protected]>
CC: [email protected]
---
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;
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 <[email protected]>
CC: Daniel Lezcano <[email protected]>
CC: Martin Schwidefsky <[email protected]>
---
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
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 <[email protected]>
CC: Michael Holzheu <[email protected]>
CC: Vivek Goyal <[email protected]>
---
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);
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 <[email protected]>
CC: Harvey Harrison <[email protected]>
---
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);
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 <[email protected]>
CC: Rashika Kheria <[email protected]>
CC: Nathan Fontenot <[email protected]>
CC: Anton Blanchard <[email protected]>
CC: Heiko Carstens <[email protected]>
CC: Yinghai Lu <[email protected]>
---
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 */
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 <[email protected]>
CC: Victor Kamensky <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: David A. Long <[email protected]>
CC: Srikar Dronamraju <[email protected]>
CC: Ananth N Mavinakayanahalli <[email protected]>
---
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 {
On Wed, Oct 15, 2014 at 10:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Daniel Lezcano <[email protected]>
> CC: Martin Schwidefsky <[email protected]>
Acked-by: John Stultz <[email protected]>
On Wed, 15 Oct 2014 11:05:41 -0600 Bjorn Helgaas <[email protected]> wrote:
> 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..
> 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?
[+cc AKASHI, Richard; sorry, I botched my "stg mail" so you weren't
included the first time]
On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: AKASHI Takahiro <[email protected]>
> CC: Richard Guy Briggs <[email protected]>
> ---
> 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 */
>
[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
weren't included the first time]
On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: David Cohen <[email protected]>
> CC: Kuppuswamy Sathyanarayanan <[email protected]>
> CC: [email protected]
> ---
> 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);
>
[+cc [email protected]]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: [email protected]
> ---
> 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;
>
>
[+cc linux-mips]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: [email protected]
> ---
> 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
>
[+cc linux-mips for real this time. sheesh]
On Wed, Oct 15, 2014 at 5:27 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc [email protected]]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: [email protected]
>> ---
>> 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;
>>
>>
[+cc linux-mips]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: [email protected]
> ---
> 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;
>
[+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.]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Daniel Lezcano <[email protected]>
> CC: Martin Schwidefsky <[email protected]>
> ---
> 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
>
[+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
included the first time]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Michael Holzheu <[email protected]>
> CC: Vivek Goyal <[email protected]>
> ---
> 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);
>
[+cc Harvey, Vineet, linux-sh; arc, sh, and x86 have kgdb_arch_pc()
definitions and could see issues from this]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Harvey Harrison <[email protected]>
> ---
> 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);
>
[+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.]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Rashika Kheria <[email protected]>
> CC: Nathan Fontenot <[email protected]>
> CC: Anton Blanchard <[email protected]>
> CC: Heiko Carstens <[email protected]>
> CC: Yinghai Lu <[email protected]>
> ---
> 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 */
>
[+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.]
On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Victor Kamensky <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: David A. Long <[email protected]>
> CC: Srikar Dronamraju <[email protected]>
> CC: Ananth N Mavinakayanahalli <[email protected]>
> ---
> 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 {
>
[+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.]
On Wed, Oct 15, 2014 at 5:31 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
> included the first time]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: Michael Holzheu <[email protected]>
>> CC: Vivek Goyal <[email protected]>
>> ---
>> 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);
>>
On Wed, Oct 15, 2014 at 10:06 AM, Bjorn Helgaas <[email protected]> wrote:
>
> 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 <[email protected]>
> CC: Harvey Harrison <[email protected]>
Reviewed-by: Harvey Harrison <[email protected]>
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 <[email protected]> wrote:
> 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 <[email protected]>
> CC: David Cohen <[email protected]>
> CC: Kuppuswamy Sathyanarayanan <[email protected]>
> CC: [email protected]
> ---
> 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 <[email protected]>
Thanks,
Ingo
* Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Daniel Lezcano <[email protected]>
> CC: Martin Schwidefsky <[email protected]>
> ---
> 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 <[email protected]>
Thanks,
Ingo
* Bjorn Helgaas <[email protected]> wrote:
> 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 <[email protected]>
> CC: Victor Kamensky <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: David A. Long <[email protected]>
> CC: Srikar Dronamraju <[email protected]>
> CC: Ananth N Mavinakayanahalli <[email protected]>
> ---
> 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 <[email protected]>
Thanks,
Ingo
* Bjorn Helgaas <[email protected]> [2014-10-15 11:06:50]:
> 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.
>
Acked-by: Srikar Dronamraju <[email protected]>
> Signed-off-by: Bjorn Helgaas <[email protected]>
> CC: Victor Kamensky <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: David A. Long <[email protected]>
> CC: Srikar Dronamraju <[email protected]>
> CC: Ananth N Mavinakayanahalli <[email protected]>
> ---
> 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
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Thanks and Regards
Srikar Dronamraju
On Wed, 15 Oct 2014 17:30:33 -0600
Bjorn Helgaas <[email protected]> wrote:
> [+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.]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> > 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 <[email protected]>
> > CC: Daniel Lezcano <[email protected]>
> > CC: Martin Schwidefsky <[email protected]>
> > ---
> > 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.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Wed, 15 Oct 2014 17:38:02 -0600
Bjorn Helgaas <[email protected]> wrote:
> [+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.]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> > 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 <[email protected]>
> > CC: Rashika Kheria <[email protected]>
> > CC: Nathan Fontenot <[email protected]>
> > CC: Anton Blanchard <[email protected]>
> > CC: Heiko Carstens <[email protected]>
> > CC: Yinghai Lu <[email protected]>
> > ---
> > 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.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Wed, 15 Oct 2014 17:44:29 -0600
Bjorn Helgaas <[email protected]> wrote:
> [+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.]
>
> On Wed, Oct 15, 2014 at 5:31 PM, Bjorn Helgaas <[email protected]> wrote:
> > [+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
> > included the first time]
> >
> > On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> >> 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 <[email protected]>
> >> CC: Michael Holzheu <[email protected]>
> >> CC: Vivek Goyal <[email protected]>
> >> ---
> >> 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.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
On Thu, Oct 16, 2014 at 1:22 AM, Martin Schwidefsky
<[email protected]> wrote:
> On Wed, 15 Oct 2014 17:30:33 -0600
> Bjorn Helgaas <[email protected]> wrote:
>
>> [+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.]
>>
>> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> > 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 <[email protected]>
>> > CC: Daniel Lezcano <[email protected]>
>> > CC: Martin Schwidefsky <[email protected]>
>> > ---
>> > 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
Hi Bjorn,
On Thursday 16 October 2014 05:05 AM, Bjorn Helgaas wrote:
> [+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
So functionally we are same as before - which is good.
If you need it.
Tested-by: Vineet Gupta <[email protected]> #for ARC build
Thx,
-Vineet
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: Harvey Harrison <[email protected]>
>> ---
>> 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);
>>
On Thu, 16 Oct 2014 07:40:16 -0600
Bjorn Helgaas <[email protected]> wrote:
> On Thu, Oct 16, 2014 at 1:22 AM, Martin Schwidefsky
> <[email protected]> wrote:
> > On Wed, 15 Oct 2014 17:30:33 -0600
> > Bjorn Helgaas <[email protected]> wrote:
> >
> >> [+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.]
> >>
> >> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> >> > 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 <[email protected]>
> >> > CC: Daniel Lezcano <[email protected]>
> >> > CC: Martin Schwidefsky <[email protected]>
> >> > ---
> >> > 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.
> 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.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
[+cc Stephen]
On Wed, Oct 15, 2014 at 5:28 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc linux-mips]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: [email protected]
>> ---
>> 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
with it:
All error/warnings:
arch/mips/kernel/vpe.c: In function 'vpe_release':
>> 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
On 14/10/15, Bjorn Helgaas wrote:
> [+cc AKASHI, Richard; sorry, I botched my "stg mail" so you weren't
> included the first time]
Makes sense, ACK.
> On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
> > 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 <[email protected]>
> > CC: AKASHI Takahiro <[email protected]>
> > CC: Richard Guy Briggs <[email protected]>
> > ---
> > 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 <[email protected]>
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
On Wed, Oct 15, 2014 at 05:44:29PM -0600, Bjorn Helgaas wrote:
> [+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.]
>
> On Wed, Oct 15, 2014 at 5:31 PM, Bjorn Helgaas <[email protected]> wrote:
> > [+cc Michael, Vivek; sorry, I botched my "stg mail" so you weren't
> > included the first time]
> >
> > On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
> >> 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 <[email protected]>
> >> CC: Michael Holzheu <[email protected]>
> >> CC: Vivek Goyal <[email protected]>
Looks good to me.
Acked-by: Vivek Goyal <[email protected]>
Thanks
Vivek
> >> ---
> >> 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);
> >>
Hi Bjorn,
On 10/15/2014 04:26 PM, Bjorn Helgaas wrote:
> [+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
> weren't included the first time]
>
> On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: David Cohen <[email protected]>
>> CC: Kuppuswamy Sathyanarayanan <[email protected]>
>> CC: [email protected]
>> ---
>> 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.
>> +++ 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
Hi Bjorn and Sathya,
On Thu, Oct 16, 2014 at 05:42:11PM -0700, sathyanarayanan kuppuswamy wrote:
> Hi Bjorn,
>
> On 10/15/2014 04:26 PM, Bjorn Helgaas wrote:
> >[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
> >weren't included the first time]
> >
> >On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
> >>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 <[email protected]>
> >>CC: David Cohen <[email protected]>
> >>CC: Kuppuswamy Sathyanarayanan <[email protected]>
> >>CC: [email protected]
> >>---
> >> 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
> >>+++ 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
>
On Thursday 16 October 2014 07:08 PM, Vineet Gupta wrote:
> On Thursday 16 October 2014 05:05 AM, Bjorn Helgaas wrote:
>> > [+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 <[email protected]>
Date: Fri, 17 Oct 2014 13:27:03 +0530
Subject: [PATCH] ARC: kgdb: generic kgdb_arch_pc() suffices
Signed-off-by: Vineet Gupta <[email protected]>
---
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
On Thu, Oct 16, 2014 at 7:41 PM, David Cohen
<[email protected]> wrote:
> Hi Bjorn and Sathya,
>
> On Thu, Oct 16, 2014 at 05:42:11PM -0700, sathyanarayanan kuppuswamy wrote:
>> Hi Bjorn,
>>
>> On 10/15/2014 04:26 PM, Bjorn Helgaas wrote:
>> >[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
>> >weren't included the first time]
>> >
>> >On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
>> >>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 <[email protected]>
>> >>CC: David Cohen <[email protected]>
>> >>CC: Kuppuswamy Sathyanarayanan <[email protected]>
>> >>CC: [email protected]
>> >>---
>> >> 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.
> 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.
[1] http://lkml.kernel.org/r/[email protected]
>> >>+++ 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
>>
On Fri, Oct 17, 2014 at 1:59 AM, Vineet Gupta
<[email protected]> wrote:
> On Thursday 16 October 2014 07:08 PM, Vineet Gupta wrote:
>> On Thursday 16 October 2014 05:05 AM, Bjorn Helgaas wrote:
>>> > [+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
> ---------------------->
> From f01b1db6986924e794eddc038167329d63d1fda9 Mon Sep 17 00:00:00 2001
> From: Vineet Gupta <[email protected]>
> Date: Fri, 17 Oct 2014 13:27:03 +0530
> Subject: [PATCH] ARC: kgdb: generic kgdb_arch_pc() suffices
>
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
> 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
On Mon, Oct 20, 2014 at 10:15:17AM -0600, Bjorn Helgaas wrote:
> On Thu, Oct 16, 2014 at 7:41 PM, David Cohen
> <[email protected]> wrote:
> > Hi Bjorn and Sathya,
> >
> > On Thu, Oct 16, 2014 at 05:42:11PM -0700, sathyanarayanan kuppuswamy wrote:
> >> Hi Bjorn,
> >>
> >> On 10/15/2014 04:26 PM, Bjorn Helgaas wrote:
> >> >[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
> >> >weren't included the first time]
> >> >
> >> >On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <[email protected]> wrote:
> >> >>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 <[email protected]>
> >> >>CC: David Cohen <[email protected]>
> >> >>CC: Kuppuswamy Sathyanarayanan <[email protected]>
> >> >>CC: [email protected]
> >> >>---
> >> >> 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.
>
> > 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.
>
> [1] http://lkml.kernel.org/r/[email protected]
Yeah, a lot late :(
Please, go ahead with this patch and I send something on top of it.
BR, David
>
>
> >> >>+++ 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
> >>
On Wed, Oct 15, 2014 at 5:28 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc linux-mips for real this time. sheesh]
>
> On Wed, Oct 15, 2014 at 5:27 PM, Bjorn Helgaas <[email protected]> wrote:
>> [+cc [email protected]]
>>
>> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>>> 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 <[email protected]>
>>> CC: [email protected]
I'm dropping this patch from my series because I haven't seen any
response from the MIPS folks.
>>> ---
>>> 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;
>>>
>>>
On Wed, Oct 15, 2014 at 5:27 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc linux-mips]
>
> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>> 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 <[email protected]>
>> CC: [email protected]
Dropping this because no MIPS folks responded.
>> ---
>> 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
>>
On Thu, Oct 16, 2014 at 7:49 AM, Bjorn Helgaas <[email protected]> wrote:
> [+cc Stephen]
>
> On Wed, Oct 15, 2014 at 5:28 PM, Bjorn Helgaas <[email protected]> wrote:
>> [+cc linux-mips]
>>
>> On Wed, Oct 15, 2014 at 11:06 AM, Bjorn Helgaas <[email protected]> wrote:
>>> 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 <[email protected]>
>>> CC: [email protected]
Dropping this permanently since I haven't heard from any MIPS folks.
>>> ---
>>> 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
> with it:
>
> All error/warnings:
>
> arch/mips/kernel/vpe.c: In function 'vpe_release':
>>> 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