| Message ID | 20220711024350.49611-1-zouyipeng@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol | expand |
On Mon, Jul 11, 2022 at 10:46 AM Yipeng Zou <zouyipeng@huawei.com> wrote: > > In riscv the process of uprobe going to clear spie before exec > the origin insn,and set spie after that.But When access the page > which origin insn has been placed a page fault may happen and > irq was disabled in arch_uprobe_pre_xol function,It cause a WARN > as follows. > There is no need to clear/set spie in arch_uprobe_pre/post/abort_xol. > We can just remove it. Seems there is no side effect on an interrupt in xol. I also checked arm64, x86, MIPS, and csky (also written by me), which are the same as yours (No irq mask). I forgot why I mask irq for userspace xol. > > [ 31.684157] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1488 > [ 31.684677] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 76, name: work > [ 31.684929] preempt_count: 0, expected: 0 > [ 31.685969] CPU: 2 PID: 76 Comm: work Tainted: G > [ 31.686542] Hardware name: riscv-virtio,qemu (DT) > [ 31.686797] Call Trace: > [ 31.687053] [<ffffffff80006442>] dump_backtrace+0x30/0x38 > [ 31.687699] [<ffffffff80812118>] show_stack+0x40/0x4c > [ 31.688141] [<ffffffff8081817a>] dump_stack_lvl+0x44/0x5c > [ 31.688396] [<ffffffff808181aa>] dump_stack+0x18/0x20 > [ 31.688653] [<ffffffff8003e454>] __might_resched+0x114/0x122 > [ 31.688948] [<ffffffff8003e4b2>] __might_sleep+0x50/0x7a > [ 31.689435] [<ffffffff80822676>] down_read+0x30/0x130 > [ 31.689728] [<ffffffff8000b650>] do_page_fault+0x166/x446 > [ 31.689997] [<ffffffff80003c0c>] ret_from_exception+0x0/0xc > > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> Please add the Fixes tag. Then: Reviewed-by: Guo Ren <guoren@kernel.org> > --- > arch/riscv/kernel/probes/uprobes.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c > index 7a057b5f0adc..c976a21cd4bd 100644 > --- a/arch/riscv/kernel/probes/uprobes.c > +++ b/arch/riscv/kernel/probes/uprobes.c > @@ -59,8 +59,6 @@ int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > > instruction_pointer_set(regs, utask->xol_vaddr); > > - regs->status &= ~SR_SPIE; > - > return 0; > } > > @@ -72,8 +70,6 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > > instruction_pointer_set(regs, utask->vaddr + auprobe->insn_size); > > - regs->status |= SR_SPIE; > - > return 0; > } > > @@ -111,8 +107,6 @@ void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > * address. > */ > instruction_pointer_set(regs, utask->vaddr); > - > - regs->status &= ~SR_SPIE; > } > > bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx, > -- > 2.17.1 >
在 2022/7/20 11:43, Guo Ren 写道: > On Mon, Jul 11, 2022 at 10:46 AM Yipeng Zou <zouyipeng@huawei.com> wrote: >> In riscv the process of uprobe going to clear spie before exec >> the origin insn,and set spie after that.But When access the page >> which origin insn has been placed a page fault may happen and >> irq was disabled in arch_uprobe_pre_xol function,It cause a WARN >> as follows. >> There is no need to clear/set spie in arch_uprobe_pre/post/abort_xol. >> We can just remove it. > Seems there is no side effect on an interrupt in xol. I also checked > arm64, x86, MIPS, and csky (also written by me), which are the same as > yours (No irq mask). I forgot why I mask irq for userspace xol. > >> [ 31.684157] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1488 >> [ 31.684677] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 76, name: work >> [ 31.684929] preempt_count: 0, expected: 0 >> [ 31.685969] CPU: 2 PID: 76 Comm: work Tainted: G >> [ 31.686542] Hardware name: riscv-virtio,qemu (DT) >> [ 31.686797] Call Trace: >> [ 31.687053] [<ffffffff80006442>] dump_backtrace+0x30/0x38 >> [ 31.687699] [<ffffffff80812118>] show_stack+0x40/0x4c >> [ 31.688141] [<ffffffff8081817a>] dump_stack_lvl+0x44/0x5c >> [ 31.688396] [<ffffffff808181aa>] dump_stack+0x18/0x20 >> [ 31.688653] [<ffffffff8003e454>] __might_resched+0x114/0x122 >> [ 31.688948] [<ffffffff8003e4b2>] __might_sleep+0x50/0x7a >> [ 31.689435] [<ffffffff80822676>] down_read+0x30/0x130 >> [ 31.689728] [<ffffffff8000b650>] do_page_fault+0x166/x446 >> [ 31.689997] [<ffffffff80003c0c>] ret_from_exception+0x0/0xc >> >> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> > Please add the Fixes tag. > > Then: > > Reviewed-by: Guo Ren <guoren@kernel.org> Allready send v2 patch(add Fixes tag) and thanks for review. >> --- >> arch/riscv/kernel/probes/uprobes.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c >> index 7a057b5f0adc..c976a21cd4bd 100644 >> --- a/arch/riscv/kernel/probes/uprobes.c >> +++ b/arch/riscv/kernel/probes/uprobes.c >> @@ -59,8 +59,6 @@ int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) >> >> instruction_pointer_set(regs, utask->xol_vaddr); >> >> - regs->status &= ~SR_SPIE; >> - >> return 0; >> } >> >> @@ -72,8 +70,6 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) >> >> instruction_pointer_set(regs, utask->vaddr + auprobe->insn_size); >> >> - regs->status |= SR_SPIE; >> - >> return 0; >> } >> >> @@ -111,8 +107,6 @@ void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) >> * address. >> */ >> instruction_pointer_set(regs, utask->vaddr); >> - >> - regs->status &= ~SR_SPIE; >> } >> >> bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx, >> -- >> 2.17.1 >> >
diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c index 7a057b5f0adc..c976a21cd4bd 100644 --- a/arch/riscv/kernel/probes/uprobes.c +++ b/arch/riscv/kernel/probes/uprobes.c @@ -59,8 +59,6 @@ int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) instruction_pointer_set(regs, utask->xol_vaddr); - regs->status &= ~SR_SPIE; - return 0; } @@ -72,8 +70,6 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) instruction_pointer_set(regs, utask->vaddr + auprobe->insn_size); - regs->status |= SR_SPIE; - return 0; } @@ -111,8 +107,6 @@ void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) * address. */ instruction_pointer_set(regs, utask->vaddr); - - regs->status &= ~SR_SPIE; } bool arch_uretprobe_is_alive(struct return_instance *ret, enum rp_check ctx,
In riscv the process of uprobe going to clear spie before exec the origin insn,and set spie after that.But When access the page which origin insn has been placed a page fault may happen and irq was disabled in arch_uprobe_pre_xol function,It cause a WARN as follows. There is no need to clear/set spie in arch_uprobe_pre/post/abort_xol. We can just remove it. [ 31.684157] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1488 [ 31.684677] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 76, name: work [ 31.684929] preempt_count: 0, expected: 0 [ 31.685969] CPU: 2 PID: 76 Comm: work Tainted: G [ 31.686542] Hardware name: riscv-virtio,qemu (DT) [ 31.686797] Call Trace: [ 31.687053] [<ffffffff80006442>] dump_backtrace+0x30/0x38 [ 31.687699] [<ffffffff80812118>] show_stack+0x40/0x4c [ 31.688141] [<ffffffff8081817a>] dump_stack_lvl+0x44/0x5c [ 31.688396] [<ffffffff808181aa>] dump_stack+0x18/0x20 [ 31.688653] [<ffffffff8003e454>] __might_resched+0x114/0x122 [ 31.688948] [<ffffffff8003e4b2>] __might_sleep+0x50/0x7a [ 31.689435] [<ffffffff80822676>] down_read+0x30/0x130 [ 31.689728] [<ffffffff8000b650>] do_page_fault+0x166/x446 [ 31.689997] [<ffffffff80003c0c>] ret_from_exception+0x0/0xc Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> --- arch/riscv/kernel/probes/uprobes.c | 6 ------ 1 file changed, 6 deletions(-)