Re: [PATCH v6 08/39] powerpc: rearrange do_page_fault error case to be inside exception_enter

[Christophe Leroy]

Le 15/01/2021 à 17:49, Nicholas Piggin a écrit :
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 <npig...@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);