Re: [git pull] x86 fixes

Linus Torvalds
2008-03-27T20:32:46Z

On Thu, 27 Mar 2008, Ingo Molnar wrote:
>
> Ingo Molnar (1):
> x86: fix prefetch workaround
...
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index fdc6674..c0c82bc 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -92,7 +92,8 @@ static int is_prefetch(struct pt_regs *regs, unsigned long addr,
> unsigned char *max_instr;
>
> #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.

- this piece of crap code is immediately followed by

/* If it was a exec fault on NX page, ignore */
if (error_code & PF_INSTR)
return 0;

which uses that *right* and readable PF_INSTR #define, and also shows
that the newly modified code is totally insane (ie: if the new code
triggers, then it would have returned 0 later _anyway_)

So I think it's just crap. I think it's duplication from the merging of
the x86 code, and I think that the fact that the new code didn't use the
right #define helper means that people didn't see that it was crap.

I pulled it, but this needs some resolution. The code makes no sense.
Should that #ifdef and the code inside of it just be removed entirely?

Linus