Messages in this thread Patch in this message | | | Date | Thu, 27 Mar 2008 21:50:12 +0100 | From | Ingo Molnar <> | Subject | Re: [git pull] x86 fixes |
| |
* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > #ifdef CONFIG_X86_32 > > - if (!(__supported_pte_mask & _PAGE_NX)) > > + /* Catch an obscure case of prefetch inside an NX page: */ > > + if ((__supported_pte_mask & _PAGE_NX) && (error_code & 16)) > > return 0; > > #endif > > Ingo, this patch makes no sense. > > Two reasons: > > - "error_code & 16" is senseless. Use PF_INSTR instead, which actually > tells the reader something.
yeah - sorry - the '16' was a blast from the past, i just took the old 2.6.24 condition on the 32-bit side which didnt use PF_INSTR (PF_INSTR came in as a cleanup during the unification), and when the confirmation came that this fixed the crash i sent the pull request. That's how this nonsensical mixing happened, and that's why i missed the 'if (error_code & PF_INSTR)' branch.
The patch below (ontop of the tree) is a first cut at fixing all these problems - but i'd wait at least 24 hours with applying this to let it be tested through - it affects both 32-bit and 64-bit. The fix further cleans up this codepath and removes an #ifdef.
Ingo
------------------------> Subject: x86: prefetch fix #2 From: Ingo Molnar <mingo@elte.hu> Date: Thu Mar 27 21:29:09 CET 2008
Linus noticed a second bug and an uncleanliness:
- we'd return on any instruction fetch fault
- we'd use both the value of 16 and the PF_INSTR symbol which are the same and make no sense
the cleanup nicely unifies this piece of logic.
Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/mm/fault.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Index: linux-x86.q/arch/x86/mm/fault.c =================================================================== --- linux-x86.q.orig/arch/x86/mm/fault.c +++ linux-x86.q/arch/x86/mm/fault.c @@ -103,14 +103,10 @@ static int is_prefetch(struct pt_regs *r int prefetch = 0; unsigned char *max_instr; -#ifdef CONFIG_X86_32 - /* Catch an obscure case of prefetch inside an NX page: */ - if ((__supported_pte_mask & _PAGE_NX) && (error_code & 16)) - return 0; -#endif - - /* If it was a exec fault on NX page, ignore */ - if (error_code & PF_INSTR) + /* + * Catch an obscure case of prefetch inside an NX page: + */ + if ((__supported_pte_mask & _PAGE_NX) && (error_code & PF_INSTR)) return 0; instr = (unsigned char *)convert_ip_to_linear(current, regs);
| |