Vincent, On Fri, 27 Sep 2019, Christoph Hellwig wrote:
> 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 <[email protected]> > > Signed-off-by: Vincent Chen <[email protected]> ... > 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"); > Christoph's suggestion looks good to me. What do you think about this modification to your patch? - Paul From: Vincent Chen <[email protected]> Date: Mon, 23 Sep 2019 08:45:17 +0800 Subject: [PATCH] riscv: remove the switch statement in do_trap_break() To make the code more straightforward, replace the switch statement with an if statement. Suggested-by: Paul Walmsley <[email protected]> Signed-off-by: Vincent Chen <[email protected]> [[email protected]: removed CONFIG_GENERIC_BUG tests per Christoph's suggestion; cleaned up patch description] Cc: Christoph Hellwig <[email protected]> Link: https://lore.kernel.org/linux-riscv/[email protected]/ Signed-off-by: Paul Walmsley <[email protected]> --- arch/riscv/kernel/traps.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 93742df9067f..45b82be00714 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -124,24 +124,13 @@ static inline unsigned long get_break_insn_length(unsigned long pc) asmlinkage void do_trap_break(struct pt_regs *regs) { - if (!user_mode(regs)) { - enum bug_trap_type type; - - type = report_bug(regs->sepc, regs); - switch (type) { -#ifdef CONFIG_GENERIC_BUG - case BUG_TRAP_TYPE_WARN: - regs->sepc += get_break_insn_length(regs->sepc); - return; - case BUG_TRAP_TYPE_BUG: -#endif /* CONFIG_GENERIC_BUG */ - default: - die(regs, "Kernel BUG"); - } - } else { + 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"); } #ifdef CONFIG_GENERIC_BUG -- 2.23.0
