linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mark Fortescue <mark@mtfhpc.demon.co.uk>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David Miller <davem@davemloft.net>,
	akpm@linux-foundation.org, linuxppc-dev@ozlabs.org,
	wli@holomorphy.com, linux-mm@kvack.org, andrea@suse.de,
	sparclinux@vger.kernel.org
Subject: Re: vm changes from linux-2.6.14 to linux-2.6.15
Date: Tue, 1 May 2007 14:58:07 +0100 (BST)	[thread overview]
Message-ID: <Pine.LNX.4.61.0705011453380.4771@mtfhpc.demon.co.uk> (raw)
In-Reply-To: <1177985136.24962.8.camel@localhost.localdomain>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 772 bytes --]



On Tue, 1 May 2007, Benjamin Herrenschmidt wrote:

>
>> At present, update_mmu_cache() and lazy_mmu_prot_update() are always
>> called when ptep_set_access_flags() is called so why not move them into
>> ptep_set_access_flags() and change ptep_set_access_flags() to have an
>> additional boolean parameter (__update) that would when set, cause
>> update_mmu_cache() and lazy_mmu_prot_update() to be called.
>
> Well, ptep_set_access_flags() is a low level arch hook, I'd rather not
> start hiding update_mmu_cache() calls in it ...
>
> Ben.
>
>

I have attached a patch (so pine does not mangle it) for linux-2.6.20.9.
Is this what you had in mind?

For linux-2.6.21, more work will be needed as it has more code calling 
ptep_set_access_flags.

Regards
 	Mark Fortescue.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10987 bytes --]

diff -ruNpd linux-2.6.20.9/include/asm-generic/pgtable.h linux-test/include/asm-generic/pgtable.h
--- linux-2.6.20.9/include/asm-generic/pgtable.h	2007-05-01 12:57:56.000000000 +0100
+++ linux-test/include/asm-generic/pgtable.h	2007-05-01 14:20:05.000000000 +0100
@@ -29,11 +29,16 @@ do {				  					\
  * to a "more permissive" setting, which allows most architectures
  * to optimize this.
  */
-#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-do {				  					  \
-	set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry);	  \
-	flush_tlb_page(__vma, __address);				  \
-} while (0)
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+({										    \
+	if (__update) {				  				    \
+		set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry);	    \
+		flush_tlb_page(__vma, __address);				    \
+	} else if (__dirty) {							    \
+		flush_tlb_page(__vma, __address);				    \
+	}									    \
+	__update;								    \
+})
 #endif
 
 #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
diff -ruNpd linux-2.6.20.9/include/asm-i386/pgtable.h linux-test/include/asm-i386/pgtable.h
--- linux-2.6.20.9/include/asm-i386/pgtable.h	2007-05-01 12:57:56.000000000 +0100
+++ linux-test/include/asm-i386/pgtable.h	2007-05-01 13:43:38.000000000 +0100
@@ -273,14 +273,17 @@ static inline pte_t pte_mkhuge(pte_t pte
  * bit at the same time.
  */
 #define  __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
-#define ptep_set_access_flags(vma, address, ptep, entry, dirty)		\
-do {									\
-	if (dirty) {							\
-		(ptep)->pte_low = (entry).pte_low;			\
-		pte_update_defer((vma)->vm_mm, (address), (ptep));	\
-		flush_tlb_page(vma, address);				\
-	}								\
-} while (0)
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update)	\
+({											\
+	if (__dirty) {									\
+		if (__update) {								\
+			(__ptep)->pte_low = (__entry).pte_low;				\
+			pte_update_defer((__vma)->vm_mm, (__address), (__ptep));	\
+		}									\
+		flush_tlb_page(__vma, __address);					\
+	}										\
+	__update;									\
+})
 
 /*
  * We don't actually have these, but we want to advertise them so that
diff -ruNpd linux-2.6.20.9/include/asm-ia64/pgtable.h linux-test/include/asm-ia64/pgtable.h
--- linux-2.6.20.9/include/asm-ia64/pgtable.h	2007-04-30 19:04:59.000000000 +0100
+++ linux-test/include/asm-ia64/pgtable.h	2007-05-01 13:48:07.000000000 +0100
@@ -537,16 +537,25 @@ extern void lazy_mmu_prot_update (pte_t 
  * daccess_bit in ivt.S).
  */
 #ifdef CONFIG_SMP
-# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable)	\
-do {											\
-	if (__safely_writable) {							\
-		set_pte(__ptep, __entry);						\
+# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __dirty, __update)	\
+({											\
+	if (__dirty) {									\
+		if (__update)								\
+			set_pte(__ptep, __entry);					\
 		flush_tlb_page(__vma, __addr);						\
 	}										\
-} while (0)
+	__update;									\
+})
 #else
-# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable)	\
-	ptep_establish(__vma, __addr, __ptep, __entry)
+# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __dirty, __update)	\
+({											\
+	if (__update)									\
+		ptep_establish(__vma, __addr, __ptep, __entry);				\
+	else if (__dirty)								\
+		flush_tlb_page(__vma, __addr);						\
+	}										\
+	__update;									\
+})
 #endif
 
 #  ifdef CONFIG_VIRTUAL_MEM_MAP
diff -ruNpd linux-2.6.20.9/include/asm-powerpc/pgtable.h linux-test/include/asm-powerpc/pgtable.h
--- linux-2.6.20.9/include/asm-powerpc/pgtable.h	2007-05-01 12:57:56.000000000 +0100
+++ linux-test/include/asm-powerpc/pgtable.h	2007-05-01 13:58:56.000000000 +0100
@@ -437,11 +437,15 @@ static inline void __ptep_set_access_fla
 	:"r" (bits), "r" (ptep), "m" (*ptep), "i" (_PAGE_BUSY)
 	:"cc");
 }
-#define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								   \
-		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
-		flush_tlb_page_nohash(__vma, __address);	       	   \
-	} while(0)
+#define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+({										     \
+	if (__update) {								     \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);		     \
+		flush_tlb_page_nohash(__vma, __address);			     \
+	} else if (__dirty)							     \
+		flush_tlb_page(__vma, __address);				     \
+	__update;								     \
+})
 
 /*
  * Macro to mark a page protection value as "uncacheable".
diff -ruNpd linux-2.6.20.9/include/asm-ppc/pgtable.h linux-test/include/asm-ppc/pgtable.h
--- linux-2.6.20.9/include/asm-ppc/pgtable.h	2007-04-30 19:05:00.000000000 +0100
+++ linux-test/include/asm-ppc/pgtable.h	2007-05-01 13:50:30.000000000 +0100
@@ -693,11 +693,15 @@ static inline void __ptep_set_access_fla
 	pte_update(ptep, 0, bits);
 }
 
-#define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								   \
-		__ptep_set_access_flags(__ptep, __entry, __dirty);	   \
-		flush_tlb_page_nohash(__vma, __address);	       	   \
-	} while(0)
+#define  ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+({										     \
+	if (__update) {								     \
+		__ptep_set_access_flags(__ptep, __entry, __dirty);		     \
+		flush_tlb_page_nohash(__vma, __address);			     \
+	} else if (__dirty)							     \
+		flush_tlb_page(__vma, __address);				     \
+	__update;								     \
+})
 
 /*
  * Macro to mark a page protection value as "uncacheable".
diff -ruNpd linux-2.6.20.9/include/asm-s390/pgtable.h linux-test/include/asm-s390/pgtable.h
--- linux-2.6.20.9/include/asm-s390/pgtable.h	2007-05-01 12:57:56.000000000 +0100
+++ linux-test/include/asm-s390/pgtable.h	2007-05-01 14:23:24.000000000 +0100
@@ -628,8 +628,14 @@ ptep_establish(struct vm_area_struct *vm
 	set_pte(ptep, entry);
 }
 
-#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	ptep_establish(__vma, __address, __ptep, __entry)
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+	({									    \
+		if (__update)							    \
+			ptep_establish(__vma, __address, __ptep, __entry);	    \
+		else if (__dirty)						    \
+			flush_tlb_page(__vma, __address);			    \
+		__update;							    \
+	})
 
 /*
  * Test and clear dirty bit in storage key.
diff -ruNpd linux-2.6.20.9/include/asm-sparc/pgtable.h linux-test/include/asm-sparc/pgtable.h
--- linux-2.6.20.9/include/asm-sparc/pgtable.h	2007-04-30 19:05:01.000000000 +0100
+++ linux-test/include/asm-sparc/pgtable.h	2007-05-01 14:32:35.000000000 +0100
@@ -446,6 +446,27 @@ extern int io_remap_pfn_range(struct vm_
 #define GET_IOSPACE(pfn)		(pfn >> (BITS_PER_LONG - 4))
 #define GET_PFN(pfn)			(pfn & 0x0fffffffUL)
 
+#define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+({										    \
+	int __ret;								    \
+										    \
+	if (sparc_cpu_model == sun4c) {						    \
+		set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry);	    \
+		flush_tlb_page(__vma, __address);				    \
+		__ret = 1;							    \
+	} else {								    \
+		if (__update) {			  				    \
+			set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry);   \
+			flush_tlb_page(__vma, __address);			    \
+		} else if (__dirty) {						    \
+			flush_tlb_page(__vma, __address);			    \
+		}								    \
+		__ret = __update;						    \
+	}									    \
+	__ret;									    \
+})
+
 #include <asm-generic/pgtable.h>
 
 #endif /* !(__ASSEMBLY__) */
diff -ruNpd linux-2.6.20.9/include/asm-x86_64/pgtable.h linux-test/include/asm-x86_64/pgtable.h
--- linux-2.6.20.9/include/asm-x86_64/pgtable.h	2007-05-01 12:57:57.000000000 +0100
+++ linux-test/include/asm-x86_64/pgtable.h	2007-05-01 13:37:08.000000000 +0100
@@ -396,13 +396,15 @@ static inline pte_t pte_modify(pte_t pte
  * race with other CPU's that might be updating the dirty
  * bit at the same time. */
 #define  __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
-#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
-	do {								  \
-		if (__dirty) {						  \
-			set_pte(__ptep, __entry);			  \
-			flush_tlb_page(__vma, __address);		  \
-		}							  \
-	} while (0)
+#define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty, __update) \
+	({									    \
+		if (__dirty) {							    \
+			if (__update)						    \
+				set_pte(__ptep, __entry);			    \
+			flush_tlb_page(__vma, __address);			    \
+		}								    \
+		__update;							    \
+	})
 
 /* Encode and de-code a swap entry */
 #define __swp_type(x)			(((x).val >> 1) & 0x3f)
diff -ruNpd linux-2.6.20.9/mm/hugetlb.c linux-test/mm/hugetlb.c
--- linux-2.6.20.9/mm/hugetlb.c	2007-05-01 13:01:10.000000000 +0100
+++ linux-test/mm/hugetlb.c	2007-05-01 13:17:26.000000000 +0100
@@ -313,9 +313,10 @@ static void set_huge_ptep_writable(struc
 	pte_t entry;
 
 	entry = pte_mkwrite(pte_mkdirty(*ptep));
-	ptep_set_access_flags(vma, address, ptep, entry, 1);
-	update_mmu_cache(vma, address, entry);
-	lazy_mmu_prot_update(entry);
+	if (ptep_set_access_flags(vma, address, ptep, entry, 1, 1)) {
+		update_mmu_cache(vma, address, entry);
+		lazy_mmu_prot_update(entry);
+	}
 }
 
 
diff -ruNpd linux-2.6.20.9/mm/memory.c linux-test/mm/memory.c
--- linux-2.6.20.9/mm/memory.c	2007-05-01 12:57:57.000000000 +0100
+++ linux-test/mm/memory.c	2007-05-01 14:26:38.000000000 +0100
@@ -1553,9 +1553,11 @@ static int do_wp_page(struct mm_struct *
 		flush_cache_page(vma, address, pte_pfn(orig_pte));
 		entry = pte_mkyoung(orig_pte);
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
-		ptep_set_access_flags(vma, address, page_table, entry, 1);
-		update_mmu_cache(vma, address, entry);
-		lazy_mmu_prot_update(entry);
+		if (ptep_set_access_flags(vma, address, page_table,
+					  entry, 1, 1)) {
+			update_mmu_cache(vma, address, entry);
+			lazy_mmu_prot_update(entry);
+		}
 		ret |= VM_FAULT_WRITE;
 		goto unlock;
 	}
@@ -2423,19 +2425,10 @@ static inline int handle_pte_fault(struc
 		entry = pte_mkdirty(entry);
 	}
 	entry = pte_mkyoung(entry);
-	if (!pte_same(old_entry, entry)) {
-		ptep_set_access_flags(vma, address, pte, entry, write_access);
+	if (ptep_set_access_flags(vma, address, pte, entry, write_access,
+				  !pte_same(old_entry, entry))) {
 		update_mmu_cache(vma, address, entry);
 		lazy_mmu_prot_update(entry);
-	} else {
-		/*
-		 * This is needed only for protection faults but the arch code
-		 * is not yet telling us if this is a protection fault or not.
-		 * This still avoids useless tlb flushes for .text page faults
-		 * with threads.
-		 */
-		if (write_access)
-			flush_tlb_page(vma, address);
 	}
 unlock:
 	pte_unmap_unlock(pte, ptl);

  reply	other threads:[~2007-05-01 13:58 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Pine.LNX.4.61.0704291345480.690@mtfhpc.demon.co.uk>
     [not found] ` <1177852457.4390.26.camel@localhost.localdomain>
     [not found]   ` <Pine.LNX.4.61.0704302159140.3178@mtfhpc.demon.co.uk>
2007-04-30 21:54     ` Andrew Morton
2007-04-30 22:04       ` David Miller, Andrew Morton
2007-04-30 22:33         ` Mark Fortescue
2007-04-30 22:42           ` David Miller, Mark Fortescue
2007-05-01  0:00         ` Benjamin Herrenschmidt
2007-05-01  0:38           ` David Miller, Benjamin Herrenschmidt
2007-05-01  1:45             ` Mark Fortescue
2007-05-01  2:05               ` Benjamin Herrenschmidt
2007-05-01 13:58                 ` Mark Fortescue [this message]
2007-05-01 21:31                   ` Benjamin Herrenschmidt
2007-05-01 23:08                     ` Mark Fortescue
2007-05-09 19:44                       ` Mark Fortescue
2007-05-09 22:48                         ` Benjamin Herrenschmidt
2007-05-10  6:19                       ` Andrew Morton
2007-05-10  6:29                         ` Benjamin Herrenschmidt
2007-05-10  7:12                           ` David Miller, Benjamin Herrenschmidt
2007-05-14 19:19                             ` Hugh Dickins
2007-05-14 21:07                               ` Benjamin Herrenschmidt
2007-05-15  6:56                                 ` Benjamin Herrenschmidt
2007-05-21 14:27                                   ` Tom "spot" Callaway
2007-05-21 22:09                                     ` Benjamin Herrenschmidt
2007-05-22  6:28                                     ` [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c Benjamin Herrenschmidt
2007-05-22 17:04                                       ` Hugh Dickins
2007-05-22 22:59                                         ` Benjamin Herrenschmidt
2007-05-22 23:04                                           ` Tom "spot" Callaway
2007-05-23  4:03                                           ` Hugh Dickins
2007-05-23  4:21                                             ` Benjamin Herrenschmidt
2007-05-22 21:52                                       ` Mark Fortescue
2007-05-22 21:53                                         ` David Miller, Mark Fortescue
2007-05-22 23:29                                           ` Benjamin Herrenschmidt
2007-05-22 23:28                                         ` Benjamin Herrenschmidt

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=Pine.LNX.4.61.0705011453380.4771@mtfhpc.demon.co.uk \
    --to=mark@mtfhpc.demon.co.uk \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@suse.de \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=wli@holomorphy.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox