[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 <npiggin at 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);
>   
>