Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
> > Fix threaded user page write memory ordering > > Thanks, I did see that, but I'm sure it must have been prompted by a > discussion or another proposed patch from IBM. Maybe I'm wrong > though. Yes, my initial proposal iirc was to smp_wmb() in set_pte() but after a discussion with Linus, we ended with open coding them. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
> Fix threaded user page write memory ordering > > Make sure we order the writes to a newly created page > with the page table update that potentially exposes the > page to another CPU. > > This is a no-op on any architecture where getting the > page table spinlock will already do the ordering (notably > x86), but other architectures can care. Ah yes, the clear_* ones I knew about as I'm the one who tracked down that bug on power :-) I though the ones Nick pointed out were different tho. Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Fri, Feb 09, 2007 at 12:41:51AM +, Hugh Dickins wrote: > On Thu, 8 Feb 2007, Nick Piggin wrote: > > Still no independent confirmation as to whether this is a problem or not. > > I'm trying to convince myself none of your patch is necessary. Probably > shall fail. But how come we've survived for years with such an issue? Well I'm almost sure that the POWER guys hit this with anonymous pages being zeroed out then added to process address space -- they would occasionally see junk (via another thread, I presume). If that were the case, then I think that a read-vs-read over a hole (for example) should be buggy in the same way. > > Updated some comments, added diffstats to patches, don't use > > __SetPageUptodate as an internal page-flags.h private function. > > Depressed by profusion of PageUptodate_UpperAndlowercasevariants. > Those rmbs, you really only want them when it says "yes", don't you? Yeah, any help with naming suggestions would be appreciated. I think we can get rid of the SetPageUptodate_nowarn variant, by making SetNewPageUptodate simply do an smp_wmb + __set_bit on all architectures? This frees us from the extra atomic ops I'd added into the page fault fastpaths as well. Yes, we do only need the rmb if it is uptodate, that's a good point. > > I would like to eventually get an ack from Hugh regarding the anon memory > > and especially swap side of the equation, > > Plea noted. I'm pondering. "Eventually" indeed. OTOH I expect you're > right to criticize anon/swap PageUptodate being set where it was needed > to get by, rather than where it was natural to do so. Well if I can get you to warm to that aspect of the patch... :) I expect that using non-atomic bitops might help. Should I make that change into its own patch? > > and a glance from whoever put the > > smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) > > From: Linus Torvalds <[EMAIL PROTECTED]> > Date: Thu, 14 Oct 2004 04:00:06 + (-0700) > Subject: Fix threaded user page write memory ordering > X-Git-Tag: v2.6.9-final~3 > X-Git-Url: > http://127.0.0.1:1234/?p=.git;a=commitdiff_plain;h=538ce05c0ef4055cf29a92a4abcdf139d180a0f9;hp=8c225dbc5a7b13801a8254aae0ccebab8e4bece7 > > Fix threaded user page write memory ordering Thanks, I did see that, but I'm sure it must have been prompted by a discussion or another proposed patch from IBM. Maybe I'm wrong though. Thanks, Nick - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Thu, 8 Feb 2007, Nick Piggin wrote: > Still no independent confirmation as to whether this is a problem or not. I'm trying to convince myself none of your patch is necessary. Probably shall fail. But how come we've survived for years with such an issue? > Updated some comments, added diffstats to patches, don't use > __SetPageUptodate as an internal page-flags.h private function. Depressed by profusion of PageUptodate_UpperAndlowercasevariants. Those rmbs, you really only want them when it says "yes", don't you? > > I would like to eventually get an ack from Hugh regarding the anon memory > and especially swap side of the equation, Plea noted. I'm pondering. "Eventually" indeed. OTOH I expect you're right to criticize anon/swap PageUptodate being set where it was needed to get by, rather than where it was natural to do so. > and a glance from whoever put the > smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) From: Linus Torvalds <[EMAIL PROTECTED]> Date: Thu, 14 Oct 2004 04:00:06 + (-0700) Subject: Fix threaded user page write memory ordering X-Git-Tag: v2.6.9-final~3 X-Git-Url: http://127.0.0.1:1234/?p=.git;a=commitdiff_plain;h=538ce05c0ef4055cf29a92a4abcdf139d180a0f9;hp=8c225dbc5a7b13801a8254aae0ccebab8e4bece7 Fix threaded user page write memory ordering Make sure we order the writes to a newly created page with the page table update that potentially exposes the page to another CPU. This is a no-op on any architecture where getting the page table spinlock will already do the ordering (notably x86), but other architectures can care. --- diff --git a/include/linux/highmem.h b/include/linux/highmem.h index 232d8fd..7153aef 100644 --- a/include/linux/highmem.h +++ b/include/linux/highmem.h @@ -40,6 +40,8 @@ static inline void clear_user_highpage(s void *addr = kmap_atomic(page, KM_USER0); clear_user_page(addr, vaddr, page); kunmap_atomic(addr, KM_USER0); + /* Make sure this page is cleared on other CPU's too before using it */ + smp_wmb(); } static inline void clear_highpage(struct page *page) @@ -73,6 +75,8 @@ static inline void copy_user_highpage(st copy_user_page(vto, vfrom, vaddr, to); kunmap_atomic(vfrom, KM_USER0); kunmap_atomic(vto, KM_USER1); + /* Make sure this page is cleared on other CPU's too before using it */ + smp_wmb(); } static inline void copy_highpage(struct page *to, struct page *from) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Thu, 2007-02-08 at 14:26 +0100, Nick Piggin wrote: > Still no independent confirmation as to whether this is a problem or not. > Updated some comments, added diffstats to patches, don't use __SetPageUptodate > as an internal page-flags.h private function. > > I would like to eventually get an ack from Hugh regarding the anon memory > and especially swap side of the equation, and a glance from whoever put the > smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) I don't remember adding that one ... Anton ? Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
Still no independent confirmation as to whether this is a problem or not. Updated some comments, added diffstats to patches, don't use __SetPageUptodate as an internal page-flags.h private function. I would like to eventually get an ack from Hugh regarding the anon memory and especially swap side of the equation, and a glance from whoever put the smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) Thanks, Nick -- SuSE Labs - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
Still no independent confirmation as to whether this is a problem or not. Updated some comments, added diffstats to patches, don't use __SetPageUptodate as an internal page-flags.h private function. I would like to eventually get an ack from Hugh regarding the anon memory and especially swap side of the equation, and a glance from whoever put the smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) Thanks, Nick -- SuSE Labs - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Thu, 2007-02-08 at 14:26 +0100, Nick Piggin wrote: Still no independent confirmation as to whether this is a problem or not. Updated some comments, added diffstats to patches, don't use __SetPageUptodate as an internal page-flags.h private function. I would like to eventually get an ack from Hugh regarding the anon memory and especially swap side of the equation, and a glance from whoever put the smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) I don't remember adding that one ... Anton ? Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Thu, 8 Feb 2007, Nick Piggin wrote: Still no independent confirmation as to whether this is a problem or not. I'm trying to convince myself none of your patch is necessary. Probably shall fail. But how come we've survived for years with such an issue? Updated some comments, added diffstats to patches, don't use __SetPageUptodate as an internal page-flags.h private function. Depressed by profusion of PageUptodate_UpperAndlowercasevariants. Those rmbs, you really only want them when it says yes, don't you? I would like to eventually get an ack from Hugh regarding the anon memory and especially swap side of the equation, Plea noted. I'm pondering. Eventually indeed. OTOH I expect you're right to criticize anon/swap PageUptodate being set where it was needed to get by, rather than where it was natural to do so. and a glance from whoever put the smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) From: Linus Torvalds [EMAIL PROTECTED] Date: Thu, 14 Oct 2004 04:00:06 + (-0700) Subject: Fix threaded user page write memory ordering X-Git-Tag: v2.6.9-final~3 X-Git-Url: http://127.0.0.1:1234/?p=.git;a=commitdiff_plain;h=538ce05c0ef4055cf29a92a4abcdf139d180a0f9;hp=8c225dbc5a7b13801a8254aae0ccebab8e4bece7 Fix threaded user page write memory ordering Make sure we order the writes to a newly created page with the page table update that potentially exposes the page to another CPU. This is a no-op on any architecture where getting the page table spinlock will already do the ordering (notably x86), but other architectures can care. --- diff --git a/include/linux/highmem.h b/include/linux/highmem.h index 232d8fd..7153aef 100644 --- a/include/linux/highmem.h +++ b/include/linux/highmem.h @@ -40,6 +40,8 @@ static inline void clear_user_highpage(s void *addr = kmap_atomic(page, KM_USER0); clear_user_page(addr, vaddr, page); kunmap_atomic(addr, KM_USER0); + /* Make sure this page is cleared on other CPU's too before using it */ + smp_wmb(); } static inline void clear_highpage(struct page *page) @@ -73,6 +75,8 @@ static inline void copy_user_highpage(st copy_user_page(vto, vfrom, vaddr, to); kunmap_atomic(vfrom, KM_USER0); kunmap_atomic(vto, KM_USER1); + /* Make sure this page is cleared on other CPU's too before using it */ + smp_wmb(); } static inline void copy_highpage(struct page *to, struct page *from) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
On Fri, Feb 09, 2007 at 12:41:51AM +, Hugh Dickins wrote: On Thu, 8 Feb 2007, Nick Piggin wrote: Still no independent confirmation as to whether this is a problem or not. I'm trying to convince myself none of your patch is necessary. Probably shall fail. But how come we've survived for years with such an issue? Well I'm almost sure that the POWER guys hit this with anonymous pages being zeroed out then added to process address space -- they would occasionally see junk (via another thread, I presume). If that were the case, then I think that a read-vs-read over a hole (for example) should be buggy in the same way. Updated some comments, added diffstats to patches, don't use __SetPageUptodate as an internal page-flags.h private function. Depressed by profusion of PageUptodate_UpperAndlowercasevariants. Those rmbs, you really only want them when it says yes, don't you? Yeah, any help with naming suggestions would be appreciated. I think we can get rid of the SetPageUptodate_nowarn variant, by making SetNewPageUptodate simply do an smp_wmb + __set_bit on all architectures? This frees us from the extra atomic ops I'd added into the page fault fastpaths as well. Yes, we do only need the rmb if it is uptodate, that's a good point. I would like to eventually get an ack from Hugh regarding the anon memory and especially swap side of the equation, Plea noted. I'm pondering. Eventually indeed. OTOH I expect you're right to criticize anon/swap PageUptodate being set where it was needed to get by, rather than where it was natural to do so. Well if I can get you to warm to that aspect of the patch... :) I expect that using non-atomic bitops might help. Should I make that change into its own patch? and a glance from whoever put the smp_wmb()s into the copy functions (Was it Ben H or Anton maybe?) From: Linus Torvalds [EMAIL PROTECTED] Date: Thu, 14 Oct 2004 04:00:06 + (-0700) Subject: Fix threaded user page write memory ordering X-Git-Tag: v2.6.9-final~3 X-Git-Url: http://127.0.0.1:1234/?p=.git;a=commitdiff_plain;h=538ce05c0ef4055cf29a92a4abcdf139d180a0f9;hp=8c225dbc5a7b13801a8254aae0ccebab8e4bece7 Fix threaded user page write memory ordering Thanks, I did see that, but I'm sure it must have been prompted by a discussion or another proposed patch from IBM. Maybe I'm wrong though. Thanks, Nick - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
Fix threaded user page write memory ordering Make sure we order the writes to a newly created page with the page table update that potentially exposes the page to another CPU. This is a no-op on any architecture where getting the page table spinlock will already do the ordering (notably x86), but other architectures can care. Ah yes, the clear_* ones I knew about as I'm the one who tracked down that bug on power :-) I though the ones Nick pointed out were different tho. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 0/3] 2.6.20 fix for PageUptodate memorder problem (try 2)
Fix threaded user page write memory ordering Thanks, I did see that, but I'm sure it must have been prompted by a discussion or another proposed patch from IBM. Maybe I'm wrong though. Yes, my initial proposal iirc was to smp_wmb() in set_pte() but after a discussion with Linus, we ended with open coding them. Ben. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
