lkml.org 
[lkml]   [2019]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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
\
 
 \ /
  Last update: 2019-10-07 18:08    [W:0.381 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site