Re: [PATCH v6 08/39] powerpc: rearrange do_page_fault error case to be inside exception_enter
From: Christophe Leroy <hidden>
Date: 2021-01-28 09:31:12
Le 15/01/2021 à 17:49, Nicholas Piggin a écrit :
quoted hunk ↗ jump to hunk
This keeps the context tracking over the entire interrupt handler which helps later with moving context tracking into interrupt wrappers. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/mm/fault.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index e476d7701413..e4121fd9fcf1 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c@@ -544,20 +544,24 @@ NOKPROBE_SYMBOL(__do_page_fault); long do_page_fault(struct pt_regs *regs) { - const struct exception_table_entry *entry; - enum ctx_state prev_state = exception_enter(); - int rc = __do_page_fault(regs, regs->dar, regs->dsisr); - exception_exit(prev_state); - if (likely(!rc)) - return 0; - - entry = search_exception_tables(regs->nip); - if (unlikely(!entry)) - return rc;
Could we keep this layout with using a 'goto' to the end of the function, instead of pushing error
handling to the right ?
Because at the end of the series once all context tracking is gone into helpers, the result looks
unfriendly.
It would look cleaner as:
static long __do_page_fault(struct pt_regs *regs)
{
long err;
const struct exception_table_entry *entry;
err = ___do_page_fault(regs, regs->dar, regs->dsisr);
if (likely(!err))
return 0;
entry = search_exception_tables(regs->nip);
if (likely(entry)) {
instruction_pointer_set(regs, extable_fixup(entry));
return 0;
} else if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64)) {
/* 32 and 64e handle this in asm */
return err;
}
__bad_page_fault(regs, err);
return 0;
}
NOKPROBE_SYMBOL(__do_page_fault);
+ enum ctx_state prev_state;
+ long err;
+
+ prev_state = exception_enter();
+ err = __do_page_fault(regs, regs->dar, regs->dsisr);
+ if (unlikely(err)) {
+ const struct exception_table_entry *entry;
+
+ entry = search_exception_tables(regs->nip);
+ if (likely(entry)) {
+ instruction_pointer_set(regs, extable_fixup(entry));
+ err = 0;
+ }
+ }
- instruction_pointer_set(regs, extable_fixup(entry));
+ exception_exit(prev_state);
- return 0;
+ return err;
}
NOKPROBE_SYMBOL(do_page_fault);