Messages in this thread | | | Date | Fri, 27 Sep 2019 15:47:11 -0700 | From | Christoph Hellwig <> | Subject | Re: [PATCH 4/4] riscv: remove the switch statement in do_trap_break() |
| |
On Mon, Sep 23, 2019 at 08:45:17AM +0800, Vincent Chen wrote: > To make the code more straightforward, replacing the switch statement > with if statement. > > Suggested-by: Paul Walmsley <paul.walmsley@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > arch/riscv/kernel/traps.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index dd13bc90aeb6..1ac75f7d0bff 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -124,23 +124,24 @@ static inline unsigned long get_break_insn_length(unsigned long pc) > > asmlinkage void do_trap_break(struct pt_regs *regs) > { > + if (user_mode(regs)) { > + force_sig_fault(SIGTRAP, TRAP_BRKPT, > + (void __user *)(regs->sepc)); > + return; > + } > +#ifdef CONFIG_GENERIC_BUG > + { > enum bug_trap_type type; > > type = report_bug(regs->sepc, regs); > + if (type == BUG_TRAP_TYPE_WARN) { > regs->sepc += get_break_insn_length(regs->sepc); > return; > } > +#endif /* CONFIG_GENERIC_BUG */ > + > + die(regs, "Kernel BUG");
I like where this is going, but I think this can be improved further given that fact that report_bug has a nice stub for the !CONFIG_GENERIC_BUG case.
How about:
if (user_mode(regs)) force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->sepc); else if (report_bug(regs->sepc, regs) == BUG_TRAP_TYPE_WARN) regs->sepc += get_break_insn_length(regs->sepc); else die(regs, "Kernel BUG");
| |