All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <compudj@krystal.dyndns.org>
To: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	ak@suse.de, mingo@redhat.com
Cc: mbligh@google.com
Subject: [PATCH] Workaround change_page_attr() and global_flush_tlb() df_list inconsistency on i386
Date: Tue, 19 Jun 2007 17:10:49 -0400	[thread overview]
Message-ID: <20070619211049.GA9234@Krystal> (raw)
In-Reply-To: <20070619200136.GA4943@Krystal>

* Mathieu Desnoyers (compudj@krystal.dyndns.org) wrote:
> Looking more closely into the code to find the cause of the
> change_page_addr()/global_flush_tlb() inconsistency, I see where the
> problem could be:
> 
> In arch/i386/mm/pageattr.c:
> __change_page_attr adds the page to the df_list for deferred removal
> when it is replaced by a large page (going back to the normal flags).
> This list is walked by global_flush_tlb(); it calls flush_map() and
> __free_page for each of these pages.
> 
> flush_map() is the only call that ends up doing a clflush/wbinvd and
> __flush_tlb_all() on every cpu. However, this is only done when there
> are pages recombined in a large page. It never happens when we set the
> page flags to something unusual in __change_page_attr().
> 
> The x86_64 implementation seems to work around this issue by doing a
> flush_map() independently of the deferred_pages list. It will therefore
> call __flush_tlb_all(), which should flush the TLB, but even there, I
> wonder if it should call clflush on the pages that had their flags
> modified by __change_page_attr() ?
> 
> Some input about the best way to fix this (adding the modified pages to
> the deferred list in __change_page_attr() or flushing all the TLBs, and
> all caches, independently of the deferred pages list in
> global_flush_tlb()) would be appreciated. If we add the pages that
> simply had their flags modified to the df_list, would it be ok to issue
> a __free_page on them ?
> 


Workaround change_page_attr() and global_flush_tlb() df_list inconsistency on i386

global_flush_tlb() does not flush the tlb of pages that had their flags changed
by change_page_attr(). It only deals with the pages that are set back to their
normal flags.

Waiting for comments about a cleaner fix, this one does the job.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/i386/mm/pageattr.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng/arch/i386/mm/pageattr.c
===================================================================
--- linux-2.6-lttng.orig/arch/i386/mm/pageattr.c	2007-06-19 15:34:14.000000000 -0400
+++ linux-2.6-lttng/arch/i386/mm/pageattr.c	2007-06-19 16:49:52.000000000 -0400
@@ -232,6 +232,8 @@
 			flush_map(page_address(pg));
 		__free_page(pg);
 	}
+	/* Workaround change page attr list missing entries */
+	flush_map(NULL);
 }
 
 #ifdef CONFIG_DEBUG_PAGEALLOC
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2007-06-20  4:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-19 17:09 Problem with global_flush_tlb() on i386 in 2.6.22-rc4-mm2 Mathieu Desnoyers
2007-06-19 20:01 ` Problem with global_flush_tlb() on i386 (x86_64? too) " Mathieu Desnoyers
2007-06-19 21:10   ` Mathieu Desnoyers [this message]
2007-06-20  9:01   ` Andi Kleen
2007-06-20 16:46     ` Mathieu Desnoyers
2007-06-20 17:53       ` Andi Kleen
2007-06-20 18:14         ` Mathieu Desnoyers
2007-06-20 19:39         ` [PATCH] fix x86_64-mm-cpa-cache-flush.patch " Mathieu Desnoyers
     [not found]           ` <20070625212553.ec2caba9.akpm@linux-foundation.org>
2007-06-29  4:20             ` Mathieu Desnoyers
2007-06-20  1:23 ` Problem with global_flush_tlb() on i386 " Anthony Liguori
2007-06-20  1:32   ` Mathieu Desnoyers
2007-06-20  1:49   ` Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070619211049.GA9234@Krystal \
    --to=compudj@krystal.dyndns.org \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbligh@google.com \
    --cc=mingo@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.