UPROBES: Remove useless __weak attribute

PATCH In review Ralf Baechle 4 participants 5 messages Oct 9, 2013 – Oct 11, 2013

Subsystems (2)

MIPS uprobes

lore

Share & tools

lore

Ralf Baechle

author

2013-10-09 12:08 UTC #0 lore raw

PATCH 1/1 UPROBES: Remove useless __weak attribute

<linux/uprobes.h> declares arch_uprobe_skip_sstep() as a weak function. But as there is no definition of generic version so when trying to build uprobes for an architecture that doesn't yet have a arch_uprobe_skip_sstep() implementation, the vmlinux will try to call arch_uprobe_skip_sstep() somehwere in Stupidhistan leading to a system crash. We rather want a proper link error so remove arch_uprobe_skip_sstep().

Signed-off-by Ralf Baechle <[email protected]>

 include/linux/uprobes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/uprobes.h +1 -1

index 2a9d75d..cec7397 100644
@@ -124,7 +124,7 @@ extern int uprobe_post_sstep_notifier(struct pt_regs *regs);
 extern int uprobe_pre_sstep_notifier(struct pt_regs *regs);
 extern void uprobe_notify_resume(struct pt_regs *regs);
 extern bool uprobe_deny_signal(void);
-extern bool __weak arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
+extern bool arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
 extern void uprobe_clear_state(struct mm_struct *mm);
 #else /* !CONFIG_UPROBES */
 struct uprobes_state {

Oleg Nesterov

maintainer

2013-10-10 11:49 UTC #1 lore raw

Quoted 18 lines · Ralf Baechle

<linux/uprobes.h> declares arch_uprobe_skip_sstep() as a weak function. But as there is no definition of generic version so when trying to build uprobes for an architecture that doesn't yet have a arch_uprobe_skip_sstep() implementation, the vmlinux will try to call arch_uprobe_skip_sstep() somehwere in Stupidhistan leading to a system crash. We rather want a proper link error so remove arch_uprobe_skip_sstep().
Signed-off-by Ralf Baechle <[email protected]>
 include/linux/uprobes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/uprobes.h +0 -0
index 2a9d75d..cec7397 100644

include/linux/uprobes.h extern int uprobe_post_sstep_notifier(struct pt_regs *regs); · line 127

Quoted 5 lines

include/linux/uprobes.h +1 -1

@@ -124,7 +124,7 @@ extern int uprobe_post_sstep_notifier(struct pt_regs *regs);
 extern int uprobe_pre_sstep_notifier(struct pt_regs *regs);
 extern void uprobe_notify_resume(struct pt_regs *regs);
 extern bool uprobe_deny_signal(void);
-extern bool __weak arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);
+extern bool arch_uprobe_skip_sstep(struct arch_uprobe *aup, struct pt_regs *regs);

Agreed. I'll take this patch, thanks.

Oleg.

Srikar Dronamraju

Acked-by on v1 1/1

2013-10-11 00:51 UTC #2 lore raw

In reply to Ralf Baechle

Quoted 9 lines · Ralf Baechle

<linux/uprobes.h> declares arch_uprobe_skip_sstep() as a weak function. But as there is no definition of generic version so when trying to build uprobes for an architecture that doesn't yet have a arch_uprobe_skip_sstep() implementation, the vmlinux will try to call arch_uprobe_skip_sstep() somehwere in Stupidhistan leading to a system crash. We rather want a proper link error so remove arch_uprobe_skip_sstep().
Signed-off-by Ralf Baechle <[email protected]>

Acked-by Srikar Dronamraju <[email protected]>

Will be nice to have another arch(mips) support for uprobes.

Signature · 2 lines

Thanks and Regards
Srikar Dronamraju

Ralf Baechle

author

2013-10-11 12:24 UTC #3 lore raw

Quoted 1 line · Srikar Dronamraju

Will be nice to have another arch(mips) support for uprobes.

It's basically ready to be merged - but it's triggering issues elsewhere in the kernel which I have to resolve first.

The short version is that the memory special mapping created by uprobe with VM_EXEC but not VM_READ permissions is not working but it's working if I add VM_READ. That should not be necessary so something deep down in the guts of arch/mips is wrong.

Ralf

David Daney

2013-10-11 22:07 UTC #4 lore raw

Quoted 4 lines · Ralf Baechle

Quoted 1 line · Srikar Dronamraju
Will be nice to have another arch(mips) support for uprobes.

Quoted 8 lines

It's basically ready to be merged - but it's triggering issues elsewhere in the kernel which I have to resolve first.
The short version is that the memory special mapping created by uprobe with VM_EXEC but not VM_READ permissions is not working but it's working if I add VM_READ. That should not be necessary so something deep down in the guts of arch/mips is wrong.

Weird, I have a testcase that explicitly tests VM_EXEC and !VM_READ that works.

Do you have more information about the failure, I can look at it in the simulator and see where it goes wrong.

Thanks,
David Daney