Re: [PATCH 05/31] x86/mm: Reduce tlb flushes from ptep_set_access_flags()

[Rik van Riel <riel@redhat.com>]

On 10/25/2012 04:17 PM, Linus Torvalds wrote:
> On Thu, Oct 25, 2012 at 5:16 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>> From: Rik van Riel <riel@redhat.com>
>>
>> @@ -306,11 +306,26 @@ int ptep_set_access_flags(struct vm_area
>>                            pte_t entry, int dirty)
>>   {
>>          int changed = !pte_same(*ptep, entry);
>> +       /*
>> +        * If the page used to be inaccessible (_PAGE_PROTNONE), or
>> +        * this call upgrades the access permissions on the same page,
>> +        * it is safe to skip the remote TLB flush.
>> +        */
>> +       bool flush_remote = false;
>> +       if (!pte_accessible(*ptep))
>> +               flush_remote = false;
>> +       else if (pte_pfn(*ptep) != pte_pfn(entry) ||
>> +                       (pte_write(*ptep) && !pte_write(entry)) ||
>> +                       (pte_exec(*ptep) && !pte_exec(entry)))
>> +               flush_remote = true;
>>
>>          if (changed && dirty) {
>
> Did anybody ever actually look at this sh*t-for-brains patch?
>
> Yeah, I'm grumpy. But I'm wasting time looking at patches that have
> new code in them that is stupid and retarded.
>
> This is the VM, guys, we don't add stupid and retarded code.
>
> LOOK at the code, for chrissake. Just look at it. And if you don't see
> why the above is stupid and retarded, you damn well shouldn't be
> touching VM code.

I agree it is pretty ugly.  However, the above patch
did get rid of a gigantic performance regression with
Peter's code.

Doing unnecessary remote TLB flushes was costing about
90% performance with specjbb on a 4 node system.

However, if we can guarantee that ptep_set_access_flags
is only ever called for pte permission _upgrades_, we
can simply get rid of the remote TLB flush on x86, and
skip the paranoia tests we are doing above.

Do we have that kind of guarantee?