Messages in this thread | | | Date | Thu, 15 May 2014 13:50:13 -0700 | From | Dave Hansen <> | Subject | Re: [PATCH] x86: skip check for spurious faults for non-present faults |
| |
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.
| |