From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [patch 2/2]: introduce fast_gup Date: Thu, 17 Apr 2008 19:23:34 +0200 Message-ID: <1208453014.7115.39.camel@twins> References: <20080328025455.GA8083@wotan.suse.de> <20080328030023.GC8083@wotan.suse.de> <1208444605.7115.2.camel@twins> <1208448768.7115.30.camel@twins> <1208450119.7115.36.camel@twins> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-arch-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: To: Linus Torvalds Cc: Nick Piggin , Andrew Morton , shaggy-V7BBcbaFuwjMbYB6QlFGEg@public.gmane.org, axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Clark Williams , Ingo Molnar , Jeremy Fitzhardinge On Thu, 2008-04-17 at 09:40 -0700, Linus Torvalds wrote: > > On Thu, 17 Apr 2008, Peter Zijlstra wrote: > > > > Here you go ;-) > > I think you should _use_ the new functions too ;) D'0h - clearly not my day today... Index: linux-2.6/arch/x86/mm/gup.c =================================================================== --- linux-2.6.orig/arch/x86/mm/gup.c +++ linux-2.6/arch/x86/mm/gup.c @@ -9,6 +9,49 @@ #include #include +#ifdef CONFIG_X86_PAE + +/* + * Companion to native_set_pte_present(); normal access takes the pte_lock + * and thus doesn't need it. + * + * This closes the race: + * + * CPU#1 CPU#2 + * ===== ===== + * + * fast_gup: + * - read low word + * + * native_set_pte_present: + * - set low word to 0 + * - set high word to new value + * + * - read high word + * + * - set low word to new value + * + */ +static inline pte_t native_get_pte(pte_t *ptep) +{ + pte_t pte; + +retry: + pte.pte_low = ptep->pte_low; + smp_rmb(); + pte.pte_high = ptep->pte_high; + smp_rmb(); + if (unlikely(pte.pte_low != ptep->pte_low)) + goto retry; + return pte; +} + +#else + +#define native_get_pte(ptep) (*(ptep)) + +#endif + /* * The performance critical leaf functions are made noinline otherwise gcc * inlines everything into a single function which results in too much @@ -36,7 +79,7 @@ static noinline int gup_pte_range(pmd_t * function that will do this properly, so it is broken on * 32-bit 3-level for the moment. */ - pte_t pte = *ptep; + pte_t pte = native_get_pte(ptep); struct page *page; if ((pte_val(pte) & mask) != result) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from pentafluge.infradead.org ([213.146.154.40]:39238 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761614AbYDQRXy (ORCPT ); Thu, 17 Apr 2008 13:23:54 -0400 Subject: Re: [patch 2/2]: introduce fast_gup From: Peter Zijlstra In-Reply-To: References: <20080328025455.GA8083@wotan.suse.de> <20080328030023.GC8083@wotan.suse.de> <1208444605.7115.2.camel@twins> <1208448768.7115.30.camel@twins> <1208450119.7115.36.camel@twins> Content-Type: text/plain Date: Thu, 17 Apr 2008 19:23:34 +0200 Message-ID: <1208453014.7115.39.camel@twins> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Linus Torvalds Cc: Nick Piggin , Andrew Morton , shaggy@austin.ibm.com, axboe@kernel.dk, linux-mm@kvack.org, linux-arch@vger.kernel.org, Clark Williams , Ingo Molnar , Jeremy Fitzhardinge Message-ID: <20080417172334.brDQCe0xP9ohpTNMnoGE5lcWREWYZc3SIoMgLcCsTB8@z> On Thu, 2008-04-17 at 09:40 -0700, Linus Torvalds wrote: > > On Thu, 17 Apr 2008, Peter Zijlstra wrote: > > > > Here you go ;-) > > I think you should _use_ the new functions too ;) D'0h - clearly not my day today... Index: linux-2.6/arch/x86/mm/gup.c =================================================================== --- linux-2.6.orig/arch/x86/mm/gup.c +++ linux-2.6/arch/x86/mm/gup.c @@ -9,6 +9,49 @@ #include #include +#ifdef CONFIG_X86_PAE + +/* + * Companion to native_set_pte_present(); normal access takes the pte_lock + * and thus doesn't need it. + * + * This closes the race: + * + * CPU#1 CPU#2 + * ===== ===== + * + * fast_gup: + * - read low word + * + * native_set_pte_present: + * - set low word to 0 + * - set high word to new value + * + * - read high word + * + * - set low word to new value + * + */ +static inline pte_t native_get_pte(pte_t *ptep) +{ + pte_t pte; + +retry: + pte.pte_low = ptep->pte_low; + smp_rmb(); + pte.pte_high = ptep->pte_high; + smp_rmb(); + if (unlikely(pte.pte_low != ptep->pte_low)) + goto retry; + return pte; +} + +#else + +#define native_get_pte(ptep) (*(ptep)) + +#endif + /* * The performance critical leaf functions are made noinline otherwise gcc * inlines everything into a single function which results in too much @@ -36,7 +79,7 @@ static noinline int gup_pte_range(pmd_t * function that will do this properly, so it is broken on * 32-bit 3-level for the moment. */ - pte_t pte = *ptep; + pte_t pte = native_get_pte(ptep); struct page *page; if ((pte_val(pte) & mask) != result)