From: Mark Fortescue <mark@mtfhpc.demon.co.uk>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linux-mm@kvack.org, wli@holomorphy.com, linuxppc-dev@ozlabs.org,
andrea@suse.de, "Tom \"spot\" Callaway" <tcallawa@redhat.com>,
sparclinux@vger.kernel.org, Hugh Dickins <hugh@veritas.com>,
akpm@linux-foundation.org, David Miller <davem@davemloft.net>
Subject: Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c
Date: Tue, 22 May 2007 22:52:13 +0100 (BST) [thread overview]
Message-ID: <Pine.LNX.4.61.0705222247010.5890@mtfhpc.demon.co.uk> (raw)
In-Reply-To: <1179815339.32247.799.camel@localhost.localdomain>
Hi Benjamin,
I have just tested this patch on my Sun4c Sparcstation 1 using my 2.6.20.9
test kernel without any problems.
Thank you for the work.
Regards
Mark Fortescue.
On Tue, 22 May 2007, Benjamin Herrenschmidt wrote:
> This patch reworks ptep_set_access_flags() and the callers so that the
> comparison to the old PTE is done inside that function, which then
> returns wether an update_mmu_cache() is needed. That allows fixing
> the sun4c situation where update_mmu_cache() needs to be forced,
> always.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> Ok, so that's only compile tested on sparc32 and powerpc 32 bits, boot
> tested on powerpc64 and not tested on others (I could use some help
> testing x86, x86_64 and s390 who also have their own implementations).
>
> Index: linux-work/include/asm-generic/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-generic/pgtable.h 2007-05-22 15:04:45.000000000 +1000
> +++ linux-work/include/asm-generic/pgtable.h 2007-05-22 15:32:21.000000000 +1000
> @@ -27,13 +27,20 @@ do { \
> * Largely same as above, but only sets the access flags (dirty,
> * accessed, and writable). Furthermore, we know it always gets set
> * to a "more permissive" setting, which allows most architectures
> - * to optimize this.
> + * to optimize this. We return wether the PTE actually changed, which
> + * in turn instructs the caller to do things like update__mmu_cache.
> + * This used to be done in the caller, but sparc needs minor faults to
> + * force that call on sun4c so we changed this macro slightly
> */
> #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)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) { \
> + set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \
> + flush_tlb_page(__vma, __address); \
> + } \
> + __changed; \
> +})
> #endif
>
> #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
> Index: linux-work/include/asm-powerpc/pgtable-ppc64.h
> ===================================================================
> --- linux-work.orig/include/asm-powerpc/pgtable-ppc64.h 2007-05-22 15:04:45.000000000 +1000
> +++ linux-work/include/asm-powerpc/pgtable-ppc64.h 2007-05-22 15:27:21.000000000 +1000
> @@ -413,10 +413,14 @@ static inline void __ptep_set_access_fla
> :"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)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) { \
> + __ptep_set_access_flags(__ptep, __entry, __dirty); \
> + flush_tlb_page_nohash(__vma, __address); \
> + } \
> + __changed; \
> +})
>
> /*
> * Macro to mark a page protection value as "uncacheable".
> Index: linux-work/mm/memory.c
> ===================================================================
> --- linux-work.orig/mm/memory.c 2007-05-22 15:04:45.000000000 +1000
> +++ linux-work/mm/memory.c 2007-05-22 15:38:19.000000000 +1000
> @@ -1691,9 +1691,10 @@ 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)) {
> + update_mmu_cache(vma, address, entry);
> + lazy_mmu_prot_update(entry);
> + }
> ret |= VM_FAULT_WRITE;
> goto unlock;
> }
> @@ -2525,10 +2526,9 @@ static inline int handle_pte_fault(struc
> pte_t *pte, pmd_t *pmd, int write_access)
> {
> pte_t entry;
> - pte_t old_entry;
> spinlock_t *ptl;
>
> - old_entry = entry = *pte;
> + entry = *pte;
> if (!pte_present(entry)) {
> if (pte_none(entry)) {
> if (vma->vm_ops) {
> @@ -2561,8 +2561,7 @@ 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)) {
> update_mmu_cache(vma, address, entry);
> lazy_mmu_prot_update(entry);
> } else {
> Index: linux-work/include/asm-powerpc/pgtable-ppc32.h
> ===================================================================
> --- linux-work.orig/include/asm-powerpc/pgtable-ppc32.h 2007-05-22 15:04:45.000000000 +1000
> +++ linux-work/include/asm-powerpc/pgtable-ppc32.h 2007-05-22 15:26:07.000000000 +1000
> @@ -673,10 +673,14 @@ static inline void __ptep_set_access_fla
> }
>
> #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)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) { \
> + __ptep_set_access_flags(__ptep, __entry, __dirty); \
> + flush_tlb_page_nohash(__vma, __address); \
> + } \
> + __changed; \
> +})
>
> /*
> * Macro to mark a page protection value as "uncacheable".
> Index: linux-work/include/asm-i386/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-i386/pgtable.h 2007-05-22 15:06:17.000000000 +1000
> +++ linux-work/include/asm-i386/pgtable.h 2007-05-22 15:16:11.000000000 +1000
> @@ -285,13 +285,15 @@ static inline pte_t native_local_ptep_ge
> */
> #define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
> #define ptep_set_access_flags(vma, address, ptep, entry, dirty) \
> -do { \
> - if (dirty) { \
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed && dirty) { \
> (ptep)->pte_low = (entry).pte_low; \
> pte_update_defer((vma)->vm_mm, (address), (ptep)); \
> flush_tlb_page(vma, address); \
> } \
> -} while (0)
> + __changed; \
> +})
>
> #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_DIRTY
> #define ptep_test_and_clear_dirty(vma, addr, ptep) ({ \
> Index: linux-work/include/asm-ppc/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-ppc/pgtable.h 2007-05-22 15:25:58.000000000 +1000
> +++ linux-work/include/asm-ppc/pgtable.h 2007-05-22 15:26:08.000000000 +1000
> @@ -694,10 +694,14 @@ static inline void __ptep_set_access_fla
> }
>
> #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)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) { \
> + __ptep_set_access_flags(__ptep, __entry, __dirty); \
> + flush_tlb_page_nohash(__vma, __address); \
> + } \
> + __changed; \
> +})
>
> /*
> * Macro to mark a page protection value as "uncacheable".
> Index: linux-work/include/asm-s390/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-s390/pgtable.h 2007-05-22 15:16:48.000000000 +1000
> +++ linux-work/include/asm-s390/pgtable.h 2007-05-22 15:20:16.000000000 +1000
> @@ -744,7 +744,12 @@ ptep_establish(struct vm_area_struct *vm
> }
>
> #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \
> - ptep_establish(__vma, __address, __ptep, __entry)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) \
> + ptep_establish(__vma, __address, __ptep, __entry); \
> + __changed; \
> +})
>
> /*
> * Test and clear dirty bit in storage key.
> Index: linux-work/include/asm-sparc/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-sparc/pgtable.h 2007-05-22 15:30:48.000000000 +1000
> +++ linux-work/include/asm-sparc/pgtable.h 2007-05-22 15:35:56.000000000 +1000
> @@ -446,6 +446,17 @@ 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) \
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed) { \
> + set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \
> + flush_tlb_page(__vma, __address); \
> + } \
> + (sparc_cpu_model == sun4c) || __changed; \
> +})
> +
> #include <asm-generic/pgtable.h>
>
> #endif /* !(__ASSEMBLY__) */
> Index: linux-work/include/asm-x86_64/pgtable.h
> ===================================================================
> --- linux-work.orig/include/asm-x86_64/pgtable.h 2007-05-22 15:20:40.000000000 +1000
> +++ linux-work/include/asm-x86_64/pgtable.h 2007-05-22 15:21:52.000000000 +1000
> @@ -395,12 +395,14 @@ static inline pte_t pte_modify(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) { \
> - set_pte(__ptep, __entry); \
> - flush_tlb_page(__vma, __address); \
> - } \
> - } while (0)
> +({ \
> + int __changed = !pte_same(*(__ptep), __entry); \
> + if (__changed && __dirty) { \
> + set_pte(__ptep, __entry); \
> + flush_tlb_page(__vma, __address); \
> + } \
> + __changed; \
> +})
>
> /* Encode and de-code a swap entry */
> #define __swp_type(x) (((x).val >> 1) & 0x3f)
>
>
>
next prev parent reply other threads:[~2007-05-22 21:52 UTC|newest]
Thread overview: 32+ 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>
2007-04-30 21:36 ` vm changes from linux-2.6.14 to linux-2.6.15 Mark Fortescue
2007-04-30 21:54 ` Andrew Morton
2007-04-30 22:04 ` David Miller
2007-04-30 22:33 ` Mark Fortescue
2007-04-30 22:42 ` David Miller
2007-05-01 0:00 ` Benjamin Herrenschmidt
2007-05-01 0:38 ` David Miller
2007-05-01 1:45 ` Mark Fortescue
2007-05-01 2:05 ` Benjamin Herrenschmidt
2007-05-01 13:58 ` Mark Fortescue
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
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 [this message]
2007-05-22 21:53 ` David Miller
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.0705222247010.5890@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=hugh@veritas.com \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=sparclinux@vger.kernel.org \
--cc=tcallawa@redhat.com \
--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;
as well as URLs for NNTP newsgroup(s).