* Re: vm changes from linux-2.6.14 to linux-2.6.15 [not found] ` <1177852457.4390.26.camel@localhost.localdomain> @ 2007-04-30 21:36 ` Mark Fortescue 2007-04-30 21:54 ` Andrew Morton 0 siblings, 1 reply; 32+ messages in thread From: Mark Fortescue @ 2007-04-30 21:36 UTC (permalink / raw) To: Andrea Arcangeli, wli, akpm; +Cc: sparclinux, linuxppc-dev Hi all, I have tracked down a failure to successfully load/run the init task on my Sparcstation 1 clone (SS1) and Sparcstation 2 (SS2), sparc32 sun4c systems, to a patch: commit 1a44e149084d772a1bcf4cdbdde8a013a8a1cfde. [PATCH] .text page fault SMP scalability optimization Removing this patch fixes the issue and allows me to use kernels later than v2.5.14. (tested using linux-2.6.20.9). Given the comment provided by the git bisect, backing out this patch will probably have undesirable conseqnences for other platforms (especially powerpc64) so, if an architecture independent solution is not available, some/all of the code in handle_pte_fault() in mm/memory.c will need be to made architecture dependent. I am not sufficiently familear with the how the SS1/SS2 mmu works and how the linux memory management system works to understand why this patch prevents my sun4c SS1/SS2 systems from working. Advice and help on the approch to take and any code changes regarding this issue would be most welcome. Regards Mark Fortescue. On Sun, 29 Apr 2007, Tom "spot" Callaway wrote: > On Sun, 2007-04-29 at 14:02 +0100, Mark Fortescue wrote: >> Hi, >> >> Does any one have or know of a good reference that would help me in >> identifing the cause of a CPU soft lockup that apears to be related to >> changes in the vertual memory handling between these two versions of the >> kernel? > > Have you tried git bisecting between the two kernels to find the > specific patch that breaks? > > ~spot > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 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 0 siblings, 1 reply; 32+ messages in thread From: Andrew Morton @ 2007-04-30 21:54 UTC (permalink / raw) To: Mark Fortescue; +Cc: sparclinux, linuxppc-dev, Andrea Arcangeli, wli, linux-mm On Mon, 30 Apr 2007 22:36:27 +0100 (BST) Mark Fortescue <mark@mtfhpc.demon.co.uk> wrote: > Hi all, > > I have tracked down a failure to successfully load/run the init task on my > Sparcstation 1 clone (SS1) and Sparcstation 2 (SS2), sparc32 sun4c > systems, to a patch: > > commit 1a44e149084d772a1bcf4cdbdde8a013a8a1cfde. > [PATCH] .text page fault SMP scalability optimization > > Removing this patch fixes the issue and allows me to use kernels later > than v2.5.14. (tested using linux-2.6.20.9). > > Given the comment provided by the git bisect, backing out this patch will > probably have undesirable conseqnences for other platforms (especially > powerpc64) so, if an architecture independent solution is not available, > some/all of the code in handle_pte_fault() in mm/memory.c will need be to > made architecture dependent. > > I am not sufficiently familear with the how the SS1/SS2 mmu works and how > the linux memory management system works to understand why this patch > prevents my sun4c SS1/SS2 systems from working. > > Advice and help on the approch to take and any code changes regarding this > issue would be most welcome. > Interesting - thanks for working that out. Let's keep linux-mm on cc please. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-04-30 21:54 ` Andrew Morton @ 2007-04-30 22:04 ` David Miller 2007-04-30 22:33 ` Mark Fortescue 2007-05-01 0:00 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 32+ messages in thread From: David Miller @ 2007-04-30 22:04 UTC (permalink / raw) To: akpm; +Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux From: Andrew Morton <akpm@linux-foundation.org> Date: Mon, 30 Apr 2007 14:54:14 -0700 > On Mon, 30 Apr 2007 22:36:27 +0100 (BST) > Mark Fortescue <mark@mtfhpc.demon.co.uk> wrote: > > > Hi all, > > > > I have tracked down a failure to successfully load/run the init task on my > > Sparcstation 1 clone (SS1) and Sparcstation 2 (SS2), sparc32 sun4c > > systems, to a patch: > > > > commit 1a44e149084d772a1bcf4cdbdde8a013a8a1cfde. > > [PATCH] .text page fault SMP scalability optimization > > > > Removing this patch fixes the issue and allows me to use kernels later > > than v2.5.14. (tested using linux-2.6.20.9). > > > > Given the comment provided by the git bisect, backing out this patch will > > probably have undesirable conseqnences for other platforms (especially > > powerpc64) so, if an architecture independent solution is not available, > > some/all of the code in handle_pte_fault() in mm/memory.c will need be to > > made architecture dependent. > > > > I am not sufficiently familear with the how the SS1/SS2 mmu works and how > > the linux memory management system works to understand why this patch > > prevents my sun4c SS1/SS2 systems from working. > > > > Advice and help on the approch to take and any code changes regarding this > > issue would be most welcome. > > > > Interesting - thanks for working that out. Let's keep linux-mm on cc please. You can't elide the update_mmu_cache() call on sun4c because that will miss some critical TLB setups which are performed there. The sun4c TLB has two tiers of entries: 1) segment maps, these hold ptes for a range of addresses 2) ptes, mapped into segment maps update_mmu_cache() on sun4c take care of allocating and setting up the segment maps, so if you elide the call this never happens and we fault forever. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 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 1 sibling, 1 reply; 32+ messages in thread From: Mark Fortescue @ 2007-04-30 22:33 UTC (permalink / raw) To: David Miller; +Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm Hi David, Is this just sun4c or does it affect other sparc32 architectures. Regards Mark Fortescue. On Mon, 30 Apr 2007, David Miller wrote: > From: Andrew Morton <akpm@linux-foundation.org> > Date: Mon, 30 Apr 2007 14:54:14 -0700 > >> On Mon, 30 Apr 2007 22:36:27 +0100 (BST) >> Mark Fortescue <mark@mtfhpc.demon.co.uk> wrote: >> >>> Hi all, >>> >>> I have tracked down a failure to successfully load/run the init task on my >>> Sparcstation 1 clone (SS1) and Sparcstation 2 (SS2), sparc32 sun4c >>> systems, to a patch: >>> >>> commit 1a44e149084d772a1bcf4cdbdde8a013a8a1cfde. >>> [PATCH] .text page fault SMP scalability optimization >>> >>> Removing this patch fixes the issue and allows me to use kernels later >>> than v2.5.14. (tested using linux-2.6.20.9). >>> >>> Given the comment provided by the git bisect, backing out this patch will >>> probably have undesirable conseqnences for other platforms (especially >>> powerpc64) so, if an architecture independent solution is not available, >>> some/all of the code in handle_pte_fault() in mm/memory.c will need be to >>> made architecture dependent. >>> >>> I am not sufficiently familear with the how the SS1/SS2 mmu works and how >>> the linux memory management system works to understand why this patch >>> prevents my sun4c SS1/SS2 systems from working. >>> >>> Advice and help on the approch to take and any code changes regarding this >>> issue would be most welcome. >>> >> >> Interesting - thanks for working that out. Let's keep linux-mm on cc please. > > You can't elide the update_mmu_cache() call on sun4c because that will > miss some critical TLB setups which are performed there. > > The sun4c TLB has two tiers of entries: > > 1) segment maps, these hold ptes for a range of addresses > 2) ptes, mapped into segment maps > > update_mmu_cache() on sun4c take care of allocating and setting > up the segment maps, so if you elide the call this never happens > and we fault forever. > - > To unsubscribe from this list: send the line "unsubscribe sparclinux" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-04-30 22:33 ` Mark Fortescue @ 2007-04-30 22:42 ` David Miller 0 siblings, 0 replies; 32+ messages in thread From: David Miller @ 2007-04-30 22:42 UTC (permalink / raw) To: mark; +Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm From: Mark Fortescue <mark@mtfhpc.demon.co.uk> Date: Mon, 30 Apr 2007 23:33:13 +0100 (BST) > Is this just sun4c or does it affect other sparc32 architectures. Only sun4c. srmmu's update_mmu_cache() is basically a NOP. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-04-30 22:04 ` David Miller 2007-04-30 22:33 ` Mark Fortescue @ 2007-05-01 0:00 ` Benjamin Herrenschmidt 2007-05-01 0:38 ` David Miller 1 sibling, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-01 0:00 UTC (permalink / raw) To: David Miller; +Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm > > Interesting - thanks for working that out. Let's keep linux-mm on cc please. > > You can't elide the update_mmu_cache() call on sun4c because that will > miss some critical TLB setups which are performed there. > > The sun4c TLB has two tiers of entries: > > 1) segment maps, these hold ptes for a range of addresses > 2) ptes, mapped into segment maps > > update_mmu_cache() on sun4c take care of allocating and setting > up the segment maps, so if you elide the call this never happens > and we fault forever. Maybe we can move that logic to ptep_set_access_flags()... in fact, the tlb flush logic should be done there too imho. There would still be the update_mmu_cache() that we don't want on powerpc in all cases I suppose. That can be done by having ptep_set_access_flags() return a boolean indicating wether update_mmu_cache() shall be called or not ... Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 0:00 ` Benjamin Herrenschmidt @ 2007-05-01 0:38 ` David Miller 2007-05-01 1:45 ` Mark Fortescue 0 siblings, 1 reply; 32+ messages in thread From: David Miller @ 2007-05-01 0:38 UTC (permalink / raw) To: benh; +Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: Tue, 01 May 2007 10:00:19 +1000 > > > > Interesting - thanks for working that out. Let's keep linux-mm on cc please. > > > > You can't elide the update_mmu_cache() call on sun4c because that will > > miss some critical TLB setups which are performed there. > > > > The sun4c TLB has two tiers of entries: > > > > 1) segment maps, these hold ptes for a range of addresses > > 2) ptes, mapped into segment maps > > > > update_mmu_cache() on sun4c take care of allocating and setting > > up the segment maps, so if you elide the call this never happens > > and we fault forever. > > Maybe we can move that logic to ptep_set_access_flags()... in fact, the > tlb flush logic should be done there too imho. > > There would still be the update_mmu_cache() that we don't want on > powerpc in all cases I suppose. That can be done by having > ptep_set_access_flags() return a boolean indicating wether > update_mmu_cache() shall be called or not ... Always doing ptep_set_access_flags() and returning a boolean like that might be a good idea. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 0:38 ` David Miller @ 2007-05-01 1:45 ` Mark Fortescue 2007-05-01 2:05 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 32+ messages in thread From: Mark Fortescue @ 2007-05-01 1:45 UTC (permalink / raw) To: David Miller; +Cc: linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm On Mon, 30 Apr 2007, David Miller wrote: > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Date: Tue, 01 May 2007 10:00:19 +1000 > >> >>>> Interesting - thanks for working that out. Let's keep linux-mm on cc please. >>> >>> You can't elide the update_mmu_cache() call on sun4c because that will >>> miss some critical TLB setups which are performed there. >>> >>> The sun4c TLB has two tiers of entries: >>> >>> 1) segment maps, these hold ptes for a range of addresses >>> 2) ptes, mapped into segment maps >>> >>> update_mmu_cache() on sun4c take care of allocating and setting >>> up the segment maps, so if you elide the call this never happens >>> and we fault forever. >> >> Maybe we can move that logic to ptep_set_access_flags()... in fact, the >> tlb flush logic should be done there too imho. >> >> There would still be the update_mmu_cache() that we don't want on >> powerpc in all cases I suppose. That can be done by having >> ptep_set_access_flags() return a boolean indicating wether >> update_mmu_cache() shall be called or not ... > > Always doing ptep_set_access_flags() and returning a boolean like > that might be a good idea. 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. On sun4c, an architecture specific function would be installed that always treats the __update parameter as set at all times. The generic function would change to somthing along the lines: #define ptep_set_access_flags(__vma, __address, \ __ptep, __entry, __dirty, __update) do { \ if (__update) { \ set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \ flush_tlb_page(__vma, __address); \ update_mmu_cache(__vma, __address, __entry); \ lazy_mmu_prot_update(__entry); \ } else if (__dirty) { flush_tlb_page(__vma, __address); \ } \ } while (0) The code in mm/memory.c and mm/hugetlb.c and the architecture specific versions of ptep_set_access_flags would then be changed acordingly. Regards Mark Fortescue. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 1:45 ` Mark Fortescue @ 2007-05-01 2:05 ` Benjamin Herrenschmidt 2007-05-01 13:58 ` Mark Fortescue 0 siblings, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-01 2:05 UTC (permalink / raw) To: Mark Fortescue Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm, David Miller > 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. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 2:05 ` Benjamin Herrenschmidt @ 2007-05-01 13:58 ` Mark Fortescue 2007-05-01 21:31 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 32+ messages in thread From: Mark Fortescue @ 2007-05-01 13:58 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm, David Miller [-- 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); ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 13:58 ` Mark Fortescue @ 2007-05-01 21:31 ` Benjamin Herrenschmidt 2007-05-01 23:08 ` Mark Fortescue 0 siblings, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-01 21:31 UTC (permalink / raw) To: Mark Fortescue Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm, David Miller > 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. I'm not 100% sure we need the 'update' argument... we can remove the whole old_entry, pte_same, etc... and just have pte_set_access_flags() read the old PTE and decide wether something needs to be changed or not. Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 21:31 ` Benjamin Herrenschmidt @ 2007-05-01 23:08 ` Mark Fortescue 2007-05-09 19:44 ` Mark Fortescue 2007-05-10 6:19 ` Andrew Morton 0 siblings, 2 replies; 32+ messages in thread From: Mark Fortescue @ 2007-05-01 23:08 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, akpm, David Miller [-- Attachment #1: Type: TEXT/PLAIN, Size: 794 bytes --] On Wed, 2 May 2007, Benjamin Herrenschmidt wrote: > >> 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. > > I'm not 100% sure we need the 'update' argument... we can remove the > whole old_entry, pte_same, etc... and just have pte_set_access_flags() > read the old PTE and decide wether something needs to be changed or not. > > Ben. > > The attached patch works on sun4c (with my simple ADA compile test) but the change in functionality may break things other platforms. The advantage of the previous patch is that the functionality is only changed for sparc sun4c so less testing would be required. Regards Mark Fortescue. [-- Attachment #2: Type: TEXT/PLAIN, Size: 10932 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 23:13:23.000000000 +0100 @@ -30,10 +30,17 @@ do { \ * 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) +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + 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 23:18:50.000000000 +0100 @@ -273,14 +273,19 @@ 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) \ +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + 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 23:16:47.000000000 +0100 @@ -537,16 +537,29 @@ 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); \ - flush_tlb_page(__vma, __addr); \ - } \ -} while (0) +# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __dirty) \ +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + if (__dirty) { \ + if (__update) \ + set_pte(__ptep, __entry); \ + flush_tlb_page(__vma, __addr); \ + } \ + __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) \ +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + 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 23:21:18.000000000 +0100 @@ -438,10 +438,16 @@ static inline void __ptep_set_access_fla :"cc"); } #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \ - do { \ +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + if (__update) { \ __ptep_set_access_flags(__ptep, __entry, __dirty); \ - flush_tlb_page_nohash(__vma, __address); \ - } while(0) + 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 23:15:29.000000000 +0100 @@ -694,10 +694,16 @@ static inline void __ptep_set_access_fla } #define ptep_set_access_flags(__vma, __address, __ptep, __entry, __dirty) \ - do { \ +({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + if (__update) { \ __ptep_set_access_flags(__ptep, __entry, __dirty); \ - flush_tlb_page_nohash(__vma, __address); \ - } while(0) + 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 23:19:58.000000000 +0100 @@ -628,8 +628,16 @@ 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) \ + ({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ + 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 23:10:49.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) \ +({ \ + int __update; \ + \ + if (sparc_cpu_model == sun4c) { \ + set_pte_at((__vma)->vm_mm, (__address), __ptep, __entry); \ + flush_tlb_page(__vma, __address); \ + __update = 1; \ + } else { \ + __update = !pte_same(*(__ptep), __entry); \ + 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; \ +}) + #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 23:13:53.000000000 +0100 @@ -397,12 +397,16 @@ 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 { \ + ({ \ + int __update = !pte_same(*(__ptep), __entry); \ + \ if (__dirty) { \ - set_pte(__ptep, __entry); \ + if (__update) \ + set_pte(__ptep, __entry); \ flush_tlb_page(__vma, __address); \ } \ - } while (0) + __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 23:02:08.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)) { + 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 23:06:21.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)) { + update_mmu_cache(vma, address, entry); + lazy_mmu_prot_update(entry); + } ret |= VM_FAULT_WRITE; goto unlock; } @@ -2387,10 +2389,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) { @@ -2423,19 +2424,9 @@ 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 { - /* - * 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); ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 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 1 sibling, 1 reply; 32+ messages in thread From: Mark Fortescue @ 2007-05-09 19:44 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, Andrew Morton, David Miller Hi Ben, Is it worth formally sending in either of my patches or does more work need to be done first? If you would like me to test any changes, it takes me app. 2 hours to cross-compile a sparc kernel for my sun4c. I use my sparc system as a diskless client with a very minimal setup to alow me to test cross compiled GCC and any small platform independent code I may be working on. I have not yet tried to get linux-2.6.21 or later working but for the test setup I have been using, it should not take too long if that is the kernel needed for testing. I may also be able to do the same testing on an embedded PowerPC (32bit) (it will need some work to get my cross compilation system working again as some kernel changes in the ppc/powerpc architechture have proven to be incompatible with my build scripts) and on x86_64/ix86. Once I have fixed the build scripts, it will take app. 4 to 6 hours to get the initial NFS root minimal system built for these additional architectures and then app. 2 hours for each test kernel build. If a simple ADA build is not considered a suficiently harsh test, then I could cross compile a specialist test application, if one is available, or compile a more extensive application (maybe gcc) on the test system. The problem of compiling a more extensive application on the sparc system is that it is a slow system running as a diskless client with its NFS root on an aging i486 over a 10MBit Ethernet. The result is it will take days to compile somthing like gcc. Regards Mark Fortescue On Wed, 2 May 2007, Mark Fortescue wrote: > > > On Wed, 2 May 2007, Benjamin Herrenschmidt wrote: > >> >>> 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. >> >> I'm not 100% sure we need the 'update' argument... we can remove the >> whole old_entry, pte_same, etc... and just have pte_set_access_flags() >> read the old PTE and decide wether something needs to be changed or not. >> >> Ben. >> >> > > The attached patch works on sun4c (with my simple ADA compile test) but the > change in functionality may break things other platforms. > > The advantage of the previous patch is that the functionality is only changed > for sparc sun4c so less testing would be required. > > Regards > Mark Fortescue. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-09 19:44 ` Mark Fortescue @ 2007-05-09 22:48 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-09 22:48 UTC (permalink / raw) To: Mark Fortescue Cc: linux-mm, wli, linuxppc-dev, andrea, sparclinux, Andrew Morton, David Miller On Wed, 2007-05-09 at 20:44 +0100, Mark Fortescue wrote: > Hi Ben, > > Is it worth formally sending in either of my patches or does more work > need to be done first? Sorry, I've been busy with other things... What do other thing about it ? Having update_mmu_cache() call buried inside the ptep_set_access_flags() sounds good ? Somebody has a better idea ? One thing I was thinking was that we could replace the whole logic with having ptep_set_access_flags() compare the new PTE bits with what was already there and return wether an update_mmu_cache() is required.... Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-01 23:08 ` Mark Fortescue 2007-05-09 19:44 ` Mark Fortescue @ 2007-05-10 6:19 ` Andrew Morton 2007-05-10 6:29 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 32+ messages in thread From: Andrew Morton @ 2007-05-10 6:19 UTC (permalink / raw) To: Mark Fortescue Cc: linuxppc-dev, wli, linux-mm, andrea, sparclinux, David Miller On Wed, 2 May 2007 00:08:31 +0100 (BST) Mark Fortescue <mark@mtfhpc.demon.co.uk> wrote: > On Wed, 2 May 2007, Benjamin Herrenschmidt wrote: > > > > >> 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. > > > > I'm not 100% sure we need the 'update' argument... we can remove the > > whole old_entry, pte_same, etc... and just have pte_set_access_flags() > > read the old PTE and decide wether something needs to be changed or not. > > > > Ben. > > > > > > The attached patch works on sun4c (with my simple ADA compile test) but > the change in functionality may break things other platforms. > > The advantage of the previous patch is that the functionality is only > changed for sparc sun4c so less testing would be required. > > Regards > Mark Fortescue. > > [Update-MMUCache-2.patch TEXT/PLAIN (10.7KB)] > 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 23:13:23.000000000 +0100 > @@ -30,10 +30,17 @@ do { \ > * 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) > +({ \ > + int __update = !pte_same(*(__ptep), __entry); \ > + \ > + 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; \ > +}) We never seemed to reach completion here? ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-10 6:19 ` Andrew Morton @ 2007-05-10 6:29 ` Benjamin Herrenschmidt 2007-05-10 7:12 ` David Miller 0 siblings, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-10 6:29 UTC (permalink / raw) To: Andrew Morton Cc: Mark Fortescue, linuxppc-dev, wli, linux-mm, andrea, sparclinux, David Miller > We never seemed to reach completion here? Well, I'm waiting for other people comments too... as I said earlier, I'm not too fan of burrying the update_mmu_cache() inside ptep_set_access_flags(), but perhaps we could remove the whole logic of reading the old PTE & comparing it, and instead have ptep_set_access_flags() do that locally and return to the caller wether a change occured that requires update_mmu_cache() to be called. That way, archs who don't actually need update_mmu_cache() under some circumstances will be able to return 0 there. What do you guys thing ? Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-10 6:29 ` Benjamin Herrenschmidt @ 2007-05-10 7:12 ` David Miller 2007-05-14 19:19 ` Hugh Dickins 0 siblings, 1 reply; 32+ messages in thread From: David Miller @ 2007-05-10 7:12 UTC (permalink / raw) To: benh; +Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: Thu, 10 May 2007 16:29:43 +1000 > > > We never seemed to reach completion here? > > Well, I'm waiting for other people comments too... as I said earlier, > I'm not too fan of burrying the update_mmu_cache() inside > ptep_set_access_flags(), but perhaps we could remove the whole logic of > reading the old PTE & comparing it, and instead have > ptep_set_access_flags() do that locally and return to the caller wether > a change occured that requires update_mmu_cache() to be called. > > That way, archs who don't actually need update_mmu_cache() under some > circumstances will be able to return 0 there. > > What do you guys thing ? I think that's a good idea. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-10 7:12 ` David Miller @ 2007-05-14 19:19 ` Hugh Dickins 2007-05-14 21:07 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 32+ messages in thread From: Hugh Dickins @ 2007-05-14 19:19 UTC (permalink / raw) To: David Miller, Benjamin Herrenschmidt Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm On Thu, 10 May 2007, David Miller wrote: > > > We never seemed to reach completion here? > > > > Well, I'm waiting for other people comments too... as I said earlier, > > I'm not too fan of burrying the update_mmu_cache() inside > > ptep_set_access_flags(), but perhaps we could remove the whole logic of > > reading the old PTE & comparing it, and instead have > > ptep_set_access_flags() do that locally and return to the caller wether > > a change occured that requires update_mmu_cache() to be called. > > > > That way, archs who don't actually need update_mmu_cache() under some > > circumstances will be able to return 0 there. > > > > What do you guys thing ? > > I think that's a good idea. I agree. Hugh ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-14 19:19 ` Hugh Dickins @ 2007-05-14 21:07 ` Benjamin Herrenschmidt 2007-05-15 6:56 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-14 21:07 UTC (permalink / raw) To: Hugh Dickins Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm, David Miller On Mon, 2007-05-14 at 20:19 +0100, Hugh Dickins wrote: > On Thu, 10 May 2007, David Miller wrote: > > > > We never seemed to reach completion here? > > > > > > Well, I'm waiting for other people comments too... as I said earlier, > > > I'm not too fan of burrying the update_mmu_cache() inside > > > ptep_set_access_flags(), but perhaps we could remove the whole logic of > > > reading the old PTE & comparing it, and instead have > > > ptep_set_access_flags() do that locally and return to the caller wether > > > a change occured that requires update_mmu_cache() to be called. > > > > > > That way, archs who don't actually need update_mmu_cache() under some > > > circumstances will be able to return 0 there. > > > > > > What do you guys thing ? > > > > I think that's a good idea. > > I agree. Ok, I'll cook a patch today. Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 2007-05-14 21:07 ` Benjamin Herrenschmidt @ 2007-05-15 6:56 ` Benjamin Herrenschmidt 2007-05-21 14:27 ` Tom "spot" Callaway 0 siblings, 1 reply; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-15 6:56 UTC (permalink / raw) To: Hugh Dickins Cc: mark, linuxppc-dev, wli, linux-mm, andrea, sparclinux, akpm, David Miller > Ok, I'll cook a patch today. Let's make it tomorrow :-( Got delayed by other urgent things Cheers, Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 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 0 siblings, 2 replies; 32+ messages in thread From: Tom "spot" Callaway @ 2007-05-21 14:27 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: mark, linux-mm, wli, linuxppc-dev, andrea, sparclinux, Hugh Dickins, akpm, David Miller On Tue, 2007-05-15 at 16:56 +1000, Benjamin Herrenschmidt wrote: > > Ok, I'll cook a patch today. > > Let's make it tomorrow :-( Got delayed by other urgent things Not to be annoying, but I'm patiently waiting on this patch. Would like to test sun4c before shipping Aurora 3.0. Any status? ~spot ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: vm changes from linux-2.6.14 to linux-2.6.15 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 1 sibling, 0 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-21 22:09 UTC (permalink / raw) To: Tom "spot" Callaway Cc: mark, linux-mm, wli, linuxppc-dev, andrea, sparclinux, Hugh Dickins, akpm, David Miller On Mon, 2007-05-21 at 09:27 -0500, Tom "spot" Callaway wrote: > On Tue, 2007-05-15 at 16:56 +1000, Benjamin Herrenschmidt wrote: > > > Ok, I'll cook a patch today. > > > > Let's make it tomorrow :-( Got delayed by other urgent things > > Not to be annoying, but I'm patiently waiting on this patch. Would like > to test sun4c before shipping Aurora 3.0. Any status? Damn, kick me harder ! I totally forgot... probably what happens when one is deep into bringing up some new HW toys ! Sorry about that, I'll do it asap. Cheers Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-21 14:27 ` Tom "spot" Callaway 2007-05-21 22:09 ` Benjamin Herrenschmidt @ 2007-05-22 6:28 ` Benjamin Herrenschmidt 2007-05-22 17:04 ` Hugh Dickins 2007-05-22 21:52 ` Mark Fortescue 1 sibling, 2 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-22 6:28 UTC (permalink / raw) To: Tom "spot" Callaway Cc: mark, linux-mm, wli, linuxppc-dev, andrea, sparclinux, Hugh Dickins, akpm, David Miller 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) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 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 21:52 ` Mark Fortescue 1 sibling, 1 reply; 32+ messages in thread From: Hugh Dickins @ 2007-05-22 17:04 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: mark, linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, akpm, David Miller 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). Looks pretty good to me. There was a minor build error in x86 (see below), and ia64 is missing (again see below). I've now built and am running this on x86, x86_64 and powerpc64; but I'm very unlikely to be doing anything which actually tickles these changes, or Andrea's original handle_pte_fault optimization. Would the "__changed && __dirty" architectures (x86, x86_64, ia64) be better off saying __changed = __dirty && pte_same? I doubt it's worth bothering about. You've updated do_wp_page to do "if (ptep_set_access_flags(...", but not updated set_huge_ptep_writable in the same way: I'd have thought you'd either leave both alone, or update them both: any reason for one not the other? But again, not really an issue. These changes came about because the sun4c needs to update_mmu_cache even in the pte_same case: might it also need to flush_tlb_page then? > --- 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 whether > + * in turn instructs the caller to do things like update__mmu_cache. 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 > */ > --- 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); \ That just needs to be: + 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; \ > +}) Here's what I think the ia64 hunk would be, unbuilt and untested. --- linux-work.orig/include/asm-ia64/pgtable.h 2007-05-13 05:41:00.000000000 +0100 +++ linux-work/include/asm-ia64/pgtable.h 2007-05-22 17:33:58.000000000 +0100 @@ -533,16 +533,23 @@ 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); \ - flush_tlb_page(__vma, __addr); \ - } \ -} while (0) +# define ptep_set_access_flags(__vma, __addr, __ptep, __entry, __safely_writable) \ +({ \ + int __changed = !pte_same(*(__ptep), __entry); \ + if (__changed && __safely_writable) { \ + set_pte(__ptep, __entry); \ + flush_tlb_page(__vma, __addr); \ + } \ + __changed; \ +}) #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, __safely_writable) \ +({ \ + int __changed = !pte_same(*(__ptep), __entry); \ + if (__changed) \ + ptep_establish(__vma, __addr, __ptep, __entry); \ + __changed; \ +}) #endif # ifdef CONFIG_VIRTUAL_MEM_MAP ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 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 0 siblings, 2 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-22 22:59 UTC (permalink / raw) To: Hugh Dickins Cc: mark, linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, akpm, David Miller > Looks pretty good to me. > > There was a minor build error in x86 (see below), and ia64 is missing > (again see below). I've now built and am running this on x86, x86_64 > and powerpc64; but I'm very unlikely to be doing anything which > actually tickles these changes, or Andrea's original handle_pte_fault > optimization. Ok. > Would the "__changed && __dirty" architectures (x86, x86_64, ia64) > be better off saying __changed = __dirty && pte_same? I doubt it's > worth bothering about. I'd say let gcc figure it out :-) > You've updated do_wp_page to do "if (ptep_set_access_flags(...", > but not updated set_huge_ptep_writable in the same way: I'd have > thought you'd either leave both alone, or update them both: any > reason for one not the other? But again, not really an issue. Nah, I must have missed set_huge_ptep_writable(). I don't think the wp code path matters much anyway, it's likely to always be different. > These changes came about because the sun4c needs to update_mmu_cache > even in the pte_same case: might it also need to flush_tlb_page then? Well, I don't know which is why I'm waiting for Tom Callaway to test. Davem mentioned update_mmu_cache only though when we discussed the problem initially. > > #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); \ > > That just needs to be: > > + int __changed = !pte_same(*(ptep), entry); \ Ah yes, sorry about that. I need to setup an x86 toolchain somewhere :-) > Here's what I think the ia64 hunk would be, unbuilt and untested. Ok. I'll respin a patch later today. Cheers, Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-22 22:59 ` Benjamin Herrenschmidt @ 2007-05-22 23:04 ` Tom "spot" Callaway 2007-05-23 4:03 ` Hugh Dickins 1 sibling, 0 replies; 32+ messages in thread From: Tom "spot" Callaway @ 2007-05-22 23:04 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: mark, linux-mm, wli, linuxppc-dev, andrea, sparclinux, Hugh Dickins, akpm, David Miller On Wed, 2007-05-23 at 08:59 +1000, Benjamin Herrenschmidt wrote: > Well, I don't know which is why I'm waiting for Tom Callaway to test. > Davem mentioned update_mmu_cache only though when we discussed the > problem initially. Mark already tested it and said it worked for him. This is sufficient for me, as I'm not sure how many Aurora sun4c users there actually are. If the extra bit turns out to be needed, I can push an update. ~spot ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 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 1 sibling, 1 reply; 32+ messages in thread From: Hugh Dickins @ 2007-05-23 4:03 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: mark, linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, akpm, David Miller On Wed, 23 May 2007, Benjamin Herrenschmidt wrote: > > > Would the "__changed && __dirty" architectures (x86, x86_64, ia64) > > be better off saying __changed = __dirty && pte_same? I doubt it's > > worth bothering about. > > I'd say let gcc figure it out :-) No, I wasn't meaning the optimization, but the significance of the boolean __changed that's returned. If ptep_set_access_flags does not change the pte (because !dirty or !safely_writable or whatever that arch calls it), then ideally it ought to return false. But it doesn't affect correctness if it sometimes says true not false, and these arches happen to have an empty update_mmu_cache (with lazy_mmu_prot_update currently under separate review), and what you have follows what was already being done, and sun4c already has to "lie": so it's rather theoretical. Hugh ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-23 4:03 ` Hugh Dickins @ 2007-05-23 4:21 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-23 4:21 UTC (permalink / raw) To: Hugh Dickins Cc: mark, linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, akpm, David Miller On Wed, 2007-05-23 at 05:03 +0100, Hugh Dickins wrote: > > No, I wasn't meaning the optimization, but the significance of the > boolean __changed that's returned. If ptep_set_access_flags does > not change the pte (because !dirty or !safely_writable or whatever > that arch calls it), then ideally it ought to return false. Hrm... I prefer keeping the existing semantics. The old code used to always update_mmu_cache() on those archs and I'd rather let it continue do so unless the arch maintainer who knows better changes it :-) > But it doesn't affect correctness if it sometimes says true not > false, and these arches happen to have an empty update_mmu_cache > (with lazy_mmu_prot_update currently under separate review), and > what you have follows what was already being done, and sun4c > already has to "lie": so it's rather theoretical. Ok. Cheers, Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 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 21:52 ` Mark Fortescue 2007-05-22 21:53 ` David Miller 2007-05-22 23:28 ` Benjamin Herrenschmidt 1 sibling, 2 replies; 32+ messages in thread From: Mark Fortescue @ 2007-05-22 21:52 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, Hugh Dickins, akpm, David Miller 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) > > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-22 21:52 ` Mark Fortescue @ 2007-05-22 21:53 ` David Miller 2007-05-22 23:29 ` Benjamin Herrenschmidt 2007-05-22 23:28 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 32+ messages in thread From: David Miller @ 2007-05-22 21:53 UTC (permalink / raw) To: mark; +Cc: linux-mm, wli, linuxppc-dev, andrea, tcallawa, sparclinux, hugh, akpm From: Mark Fortescue <mark@mtfhpc.demon.co.uk> Date: Tue, 22 May 2007 22:52:13 +0100 (BST) > 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. Thanks for your testing. Someone please merge this in once any remaining issues have been resolved :-) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-22 21:53 ` David Miller @ 2007-05-22 23:29 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-22 23:29 UTC (permalink / raw) To: David Miller Cc: mark, linux-mm, wli, linuxppc-dev, andrea, tcallawa, sparclinux, hugh, akpm On Tue, 2007-05-22 at 14:53 -0700, David Miller wrote: > From: Mark Fortescue <mark@mtfhpc.demon.co.uk> > Date: Tue, 22 May 2007 22:52:13 +0100 (BST) > > > 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. > > Thanks for your testing. > > Someone please merge this in once any remaining issues have > been resolved :-) I'll send a patch fixing the couple of x86/ia64 nits & my bad english spelling (no, I don't happen to raise castrated rams) as soon as I reach the office later today. Cheers, Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH/RFC] Rework ptep_set_access_flags and fix sun4c 2007-05-22 21:52 ` Mark Fortescue 2007-05-22 21:53 ` David Miller @ 2007-05-22 23:28 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 32+ messages in thread From: Benjamin Herrenschmidt @ 2007-05-22 23:28 UTC (permalink / raw) To: Mark Fortescue Cc: linux-mm, wli, linuxppc-dev, andrea, Tom "spot" Callaway, sparclinux, Hugh Dickins, akpm, David Miller On Tue, 2007-05-22 at 22:52 +0100, Mark Fortescue wrote: > 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. Wow, there is more than one user of these still ! :-) Cheers, Ben. ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2007-05-23 4:22 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[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
2007-05-22 21:53 ` David Miller
2007-05-22 23:29 ` Benjamin Herrenschmidt
2007-05-22 23:28 ` Benjamin Herrenschmidt
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).