Messages in this thread Patch in this message | | | Date | Mon, 7 Oct 2019 09:08:23 -0700 (PDT) | From | Paul Walmsley <> | Subject | Re: [PATCH 4/4] riscv: remove the switch statement in do_trap_break() |
| |
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 <paul.walmsley@sifive.com> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
...
> 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 <vincent.chen@sifive.com> 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 <paul.walmsley@sifive.com> Signed-off-by: Vincent Chen <vincent.chen@sifive.com> [paul.walmsley@sifive.com: removed CONFIG_GENERIC_BUG tests per Christoph's suggestion; cleaned up patch description] Cc: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/linux-riscv/20190927224711.GI4700@infradead.org/ Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> --- 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
| |