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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
