From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751827Ab2LUJ6U (ORCPT ); Fri, 21 Dec 2012 04:58:20 -0500 Received: from us01smtp3.synopsys.com ([198.182.44.81]:41632 "EHLO hermes.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751114Ab2LUJ6N (ORCPT ); Fri, 21 Dec 2012 04:58:13 -0500 Message-ID: <50D43270.8010506@synopsys.com> Date: Fri, 21 Dec 2012 15:27:04 +0530 From: Vineet Gupta User-Agent: Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20121011 Thunderbird/16.0.1 MIME-Version: 1.0 To: Rik van Riel CC: Linus Torvalds , Ingo Molnar , Andi Kleen , Michel Lespinasse , Peter Zijlstra , "Andrea Arcangeli" , Mel Gorman , "Johannes Weiner" , Thomas Gleixner , "Andrew Morton" , , , Gilad Ben Yossef , Andrea Arcangeli Subject: trailing flush_tlb_fix_spurious_fault in handle_pte_fault (was Re: [PATCH 1/3] x86/mm: only do a local TLB flush in ptep_set_access_flags()) References: <20121025121617.617683848@chello.nl> <20121025124832.840241082@chello.nl> <5089F5B5.1050206@redhat.com> <508A0A0D.4090001@redhat.com> <508A8D31.9000106@redhat.com> <20121026132601.GC9886@gmail.com> <20121026144419.7e666023@dull> <508AE1A3.6030607@redhat.com> <508E9F5B.5010402@redhat.com> In-Reply-To: <508E9F5B.5010402@redhat.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.12.197.205] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 29 October 2012 08:53 PM, Rik van Riel wrote: > On 10/26/2012 03:18 PM, Linus Torvalds wrote: >> On Fri, Oct 26, 2012 at 12:16 PM, Rik van Riel wrote: >>> >>> I can change the text of the changelog, however it looks >>> like do_wp_page does actually use ptep_set_access_flags >>> to set the write bit in the pte... >>> >>> I guess both need to be reflected in the changelog text >>> somehow? >> >> Yeah, and by now, after all this discussion, I suspect it should be >> committed with a comment too. Commit messages are good and all, but >> unless chasing a particular bug they introduced, we shouldn't expect >> people to read them for background information. > > Now that we have the TLB things taken care of, and > comments to patches 10/31 and 26/31 have been addressed, > is there anything else that needs to be done before > these NUMA patches can be merged? > > Anyone, this is a good time to speak up. We have some > time to address whatever concern you may have. > Hi, I know I'm very late in speaking up - but still I'll hazard a try. This is not exactly the same topic but closely related. There is a a different call to flush_tlb_fix_spurious( ), towards the end of handle_pte_fault( ) which commit 61c77326d "x86, mm: Avoid unnecessary TLB flush" made no-op for X86. However is this really needed for any arch at all - even if we don't know all the arch specific quirks. Given the code flow below handle_pte_fault( ) .... .... if ptep_set_access_flags()-> if PTE chg remote TLB shot (pgtable-generic.c ver) update_mmu_cache -> if PTE chg local TLB possibly shot too else flush_tlb_fix_spurious_fault -> PTE didn't change - still remote TLB shotdown So for PTE unchanged case, we default to doing remote TLB IPIs (barring X86) - unless arch makes this macro NULL. Thing is, in case of SMP races - due to PTE being different - any fixups to local/remote will be handled within ptep_set_access_flags( ) - arch-specific or generic versions. What I fail to understand is need to do anything - specially a remote shootdown, for PTE not changed case. I could shut up and just make it NO-OP for ARC, but .... Please note that for the record, the addition of this special case was done via following change. It might help answer what I feel to comprehend. 2005-10-29 1a44e14 [PATCH] .text page fault SMP scalability optimization I might be totally off track so please feel free to bash me - but atleast I would end up knowing more ! Thx, -Vineet From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx179.postini.com [74.125.245.179]) by kanga.kvack.org (Postfix) with SMTP id 596656B005A for ; Fri, 21 Dec 2012 04:58:13 -0500 (EST) Message-ID: <50D43270.8010506@synopsys.com> Date: Fri, 21 Dec 2012 15:27:04 +0530 From: Vineet Gupta MIME-Version: 1.0 Subject: trailing flush_tlb_fix_spurious_fault in handle_pte_fault (was Re: [PATCH 1/3] x86/mm: only do a local TLB flush in ptep_set_access_flags()) References: <20121025121617.617683848@chello.nl> <20121025124832.840241082@chello.nl> <5089F5B5.1050206@redhat.com> <508A0A0D.4090001@redhat.com> <508A8D31.9000106@redhat.com> <20121026132601.GC9886@gmail.com> <20121026144419.7e666023@dull> <508AE1A3.6030607@redhat.com> <508E9F5B.5010402@redhat.com> In-Reply-To: <508E9F5B.5010402@redhat.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Rik van Riel Cc: Linus Torvalds , Ingo Molnar , Andi Kleen , Michel Lespinasse , Peter Zijlstra , Andrea Arcangeli , Mel Gorman , Johannes Weiner , Thomas Gleixner , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Gilad Ben Yossef Andrea Arcangeli On Monday 29 October 2012 08:53 PM, Rik van Riel wrote: > On 10/26/2012 03:18 PM, Linus Torvalds wrote: >> On Fri, Oct 26, 2012 at 12:16 PM, Rik van Riel wrote: >>> >>> I can change the text of the changelog, however it looks >>> like do_wp_page does actually use ptep_set_access_flags >>> to set the write bit in the pte... >>> >>> I guess both need to be reflected in the changelog text >>> somehow? >> >> Yeah, and by now, after all this discussion, I suspect it should be >> committed with a comment too. Commit messages are good and all, but >> unless chasing a particular bug they introduced, we shouldn't expect >> people to read them for background information. > > Now that we have the TLB things taken care of, and > comments to patches 10/31 and 26/31 have been addressed, > is there anything else that needs to be done before > these NUMA patches can be merged? > > Anyone, this is a good time to speak up. We have some > time to address whatever concern you may have. > Hi, I know I'm very late in speaking up - but still I'll hazard a try. This is not exactly the same topic but closely related. There is a a different call to flush_tlb_fix_spurious( ), towards the end of handle_pte_fault( ) which commit 61c77326d "x86, mm: Avoid unnecessary TLB flush" made no-op for X86. However is this really needed for any arch at all - even if we don't know all the arch specific quirks. Given the code flow below handle_pte_fault( ) .... .... if ptep_set_access_flags()-> if PTE chg remote TLB shot (pgtable-generic.c ver) update_mmu_cache -> if PTE chg local TLB possibly shot too else flush_tlb_fix_spurious_fault -> PTE didn't change - still remote TLB shotdown So for PTE unchanged case, we default to doing remote TLB IPIs (barring X86) - unless arch makes this macro NULL. Thing is, in case of SMP races - due to PTE being different - any fixups to local/remote will be handled within ptep_set_access_flags( ) - arch-specific or generic versions. What I fail to understand is need to do anything - specially a remote shootdown, for PTE not changed case. I could shut up and just make it NO-OP for ARC, but .... Please note that for the record, the addition of this special case was done via following change. It might help answer what I feel to comprehend. 2005-10-29 1a44e14 [PATCH] .text page fault SMP scalability optimization I might be totally off track so please feel free to bash me - but atleast I would end up knowing more ! Thx, -Vineet -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org