Re: [PATCH] x86: skip check for spurious faults for non-present faults

[Dave Hansen <dave.hansen@intel.com>]

On 05/12/2014 03:29 AM, David Vrabel wrote:
> -	/* Reserved-bit violation or user access to kernel space? */
> -	if (error_code & (PF_USER | PF_RSVD))
> +	/* Only check for spurious faults on supervisor write or
> +	   instruction faults. */
> +	if (error_code != (PF_WRITE | PF_PROT)
> +	    && error_code != (PF_INSTR | PF_PROT))
>  		return 0;

This changes the semantics a bit too much for me to feel happy about it.
 This is at best missing quite a bit of detail from the changelog.

 1. 'return 0' means "this was not a spurious fault"
 2. We used to check for the presence of PF_USER|PF_RSVD
 3. This patch checks now for two _explicit_ conditions, which
    implicitly check for the _absence_ of the two bits we checked for
    before.

I do believe your patch is correct, but it took me a bit to convince
myself that it was the right thing.  Please be explicit (in the comment)
about the exact PTE transitions that you expect to get you here.

Also, I have to wonder if you can just leave the original if() in there.
 You're making this _more_ restrictive than it was before, and I wonder
if it might just be more clear if you have both checks.  The compiler
might even compile it down to the same code, just changing the immediate
that was generated for the mask that you're checking.