Message ID | 20220720093447.245585-1-zouyipeng@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [-next,v2] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol | expand |
On 20/07/2022 10:34, Yipeng Zou wrote: > [PATCH -next v2] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol Hey Yipeng, Just a suggestion on the commit message, how about just: "riscv: uprobe: fix SR_SPIE set/clear handling" Thanks, Conor. > [You don't often get email from zouyipeng@huawei.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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 > > Fixes: 74784081aac8 ("riscv: Add uprobes supported") > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> > --- > v2: Add Fixes tag > > 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 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 20/07/2022 10:51, Conor.Dooley@microchip.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 20/07/2022 10:34, Yipeng Zou wrote: >> [PATCH -next v2] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol > > Hey Yipeng, > Just a suggestion on the commit message, how about just: > "riscv: uprobe: fix SR_SPIE set/clear handling" Hmm also, you specified "-next" but this seems to be fixing a bug that is in current code? Should this not be applied to v5.19 too? Thanks, Conor.
在 2022/7/20 17:51, Conor.Dooley@microchip.com 写道: > On 20/07/2022 10:34, Yipeng Zou wrote: >> [PATCH -next v2] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol > Hey Yipeng, > Just a suggestion on the commit message, how about just: > "riscv: uprobe: fix SR_SPIE set/clear handling" > > Thanks, > Conor. oh, it's more reasonable, will rename in next version, thanks for advice. >> [You don't often get email from zouyipeng@huawei.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] >> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> 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 >> >> Fixes: 74784081aac8 ("riscv: Add uprobes supported") >> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> >> --- >> v2: Add Fixes tag >> >> 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 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
在 2022/7/20 17:54, Conor.Dooley@microchip.com 写道: > On 20/07/2022 10:51, Conor.Dooley@microchip.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 20/07/2022 10:34, Yipeng Zou wrote: >>> [PATCH -next v2] riscv:uprobe remove set/clear SR_SPIE in arch_uprobe_pre/post/abort_xol >> Hey Yipeng, >> Just a suggestion on the commit message, how about just: >> "riscv: uprobe: fix SR_SPIE set/clear handling" > Hmm also, you specified "-next" but this seems to be fixing a bug > that is in current code? Should this not be applied to v5.19 too? > Thanks, > Conor. Yes it is also shoifuld be applied to v5.19, It's just my local template to specied "-next" , will remove it in next version ,thanks again.
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 Fixes: 74784081aac8 ("riscv: Add uprobes supported") Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> --- v2: Add Fixes tag arch/riscv/kernel/probes/uprobes.c | 6 ------ 1 file changed, 6 deletions(-)