lkml.org 
[lkml]   [2012]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 05/31] x86/mm: Reduce tlb flushes from ptep_set_access_flags()
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?



\
 
 \ /
  Last update: 2012-10-26 05:01    [W:0.407 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site