Re: [1/2] powerpc/mm: Remove duplicated check in do_page_fault()

[Michael Ellerman <mpe@ellerman.id.au>]

On Fri, 2016-26-02 at 00:26:25 UTC, Gavin Shan wrote:
> When the page fault happened in user space, we need check it's
> caused by stack frame pointer update instruction and update
> local variable @flag with FAULT_FLAG_USER. Currently, the code
> has two separate check for the same condition. That's unnecessary.
> 
> This removes one of the duplicated check. No functinal changes
> introduced.

It's possible though that store_updates_sp() changes regs, and causes
user_mode(regs) to change, which would mean the second check is necessary.
That's not true with the current code, but you should mention that you confirmed
that in the change log.

> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index a67c6d7..935f386 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -294,11 +294,10 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
>  	 * can result in fault, which will cause a deadlock when called with
>  	 * mmap_sem held
>  	 */
> -	if (user_mode(regs))
> -		store_update_sp = store_updates_sp(regs);
> -
> -	if (user_mode(regs))
> +	if (user_mode(regs)) {
>  		flags |= FAULT_FLAG_USER;
> +		store_update_sp = store_updates_sp(regs);
> +	}

It doesn't really matter in this case, but it would be better to keep the
ordering of the two statements the same as before.

cheers