Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Benjamin Herrenschmidt
On Mon, 2007-09-24 at 14:42 -0700, Christoph Lameter wrote: > On Tue, 25 Sep 2007, Benjamin Herrenschmidt wrote: > > > I'd suggest just reverting the patch for now (well, I see from the > > commit list that you did just that) and I'll try to come up with > > something better. > > That would be

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Christoph Lameter
On Tue, 25 Sep 2007, Benjamin Herrenschmidt wrote: > I'd suggest just reverting the patch for now (well, I see from the > commit list that you did just that) and I'll try to come up with > something better. That would be great. Note that the reversal of the x86_64 quicklist support patch does

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Benjamin Herrenschmidt
On Fri, 2007-09-21 at 12:21 -0700, Linus Torvalds wrote: > Yeah, and the whole thing seems totally bogus. It totally depends on > mmu_gather doing everything right (which very much includes the dependency > on mmu gathering disabling preempt). > > For exmaple, if we were to go back to the

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Benjamin Herrenschmidt
On Fri, 2007-09-21 at 12:21 -0700, Linus Torvalds wrote: Yeah, and the whole thing seems totally bogus. It totally depends on mmu_gather doing everything right (which very much includes the dependency on mmu gathering disabling preempt). For exmaple, if we were to go back to the original

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Christoph Lameter
On Tue, 25 Sep 2007, Benjamin Herrenschmidt wrote: I'd suggest just reverting the patch for now (well, I see from the commit list that you did just that) and I'll try to come up with something better. That would be great. Note that the reversal of the x86_64 quicklist support patch does not

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-24 Thread Benjamin Herrenschmidt
On Mon, 2007-09-24 at 14:42 -0700, Christoph Lameter wrote: On Tue, 25 Sep 2007, Benjamin Herrenschmidt wrote: I'd suggest just reverting the patch for now (well, I see from the commit list that you did just that) and I'll try to come up with something better. That would be great.

RE: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Luck, Tony wrote: > > The quicklists have a long history and this bug has therefore also been in > > IA64 for a long time and it also likely exists on sparc64, sh and sh64. We > > need the patch that I posted to fix the other platforms. And with this fix > > there would be

RE: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Luck, Tony
> The quicklists have a long history and this bug has therefore also been in > IA64 for a long time and it also likely exists on sparc64, sh and sh64. We > need the patch that I posted to fix the other platforms. And with this fix > there would be nothing amiss on x86_64 either. IA64 doesn't

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: > The problem with that commit that I just reverted was that it mixed the > two, but not completely. It still kept them separate. The quicklists have a long history and this bug has therefore also been in IA64 for a long time and it also likely exists

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Linus Torvalds wrote: > > So for example, if you actually want to use quicklists in the "generic" > TLB-gather implementation, you should replace the > > struct page *pages[FREE_PTE_NR]; > > entry in the "struct mmu_gather" with a set of quicklists instead. Side

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: > > The only way to fix it would be to integrate the quicklist stuff *with* > > the mmu_gather stuff, so that these kinds of implementation issues are > > explicitly shown in the relationship, instead of havign two "independent" > > pieces of code

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: > Yeah, and the whole thing seems totally bogus. It totally depends on > mmu_gather doing everything right (which very much includes the dependency > on mmu gathering disabling preempt). The quicklists have been existing for a long time in this

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: > > The final flush does only check if there are too many pages on the > quicklists. Otherwise the quicklist is not dumped/freed (unlike the > mmu_gather list) but kept for the following page table page allocations > since we have cache hot cpu

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: > On Fri, 21 Sep 2007, Linus Torvalds wrote: > > > > On Fri, 21 Sep 2007, Christoph Lameter wrote: > > > > > > The quicklists collect pages while we gather pages for TLB flushing. > > > These pages must be kept until the actual TLB flush has occurred.

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Linus Torvalds wrote: > > On Fri, 21 Sep 2007, Christoph Lameter wrote: > > > > The quicklists collect pages while we gather pages for TLB flushing. > > These pages must be kept until the actual TLB flush has occurred. The > > optimization of releasing off node pages

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: > > The quicklists collect pages while we gather pages for TLB flushing. > These pages must be kept until the actual TLB flush has occurred. The > optimization of releasing off node pages early is therefore not valid. That should be the

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: > > Can we revert this commit for 2.6.23 and look at this code post 2.6.23? > > I'll happily revert it, but I want to understand this better, so more of > an explanation of the codepath that actually does something wrong, please. The quicklists

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Siddha, Suresh B wrote: > Essentially quicklist free routines are doing something like > __quicklist_free() > ... > if (unlikely(nid != numa_node_id())) { > __free_page(page); > ... > } > > > > > Now this will

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Siddha, Suresh B wrote: > > git commit 34feb2c83beb3bdf13535a36770f7e50b47ef299 started using quicklists > for freeing page table pages and removed the usage of tlb_remove_page() > > And looking at quicklist_free() and quicklist_free_page(), on a NUMA platform, > this can

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Andi Kleen
> Can we revert this commit for 2.6.23 and look at this code post 2.6.23? Fine by me to revert it. I must admit I never 100% understood how it was supposed to work anyways, but eventually gave up on it. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Andi Kleen
Can we revert this commit for 2.6.23 and look at this code post 2.6.23? Fine by me to revert it. I must admit I never 100% understood how it was supposed to work anyways, but eventually gave up on it. -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: The quicklists collect pages while we gather pages for TLB flushing. These pages must be kept until the actual TLB flush has occurred. The optimization of releasing off node pages early is therefore not valid. That should be the mmu_gather

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Siddha, Suresh B wrote: Essentially quicklist free routines are doing something like __quicklist_free() ... if (unlikely(nid != numa_node_id())) { __free_page(page); ... } Now this will potentially cause

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Siddha, Suresh B wrote: git commit 34feb2c83beb3bdf13535a36770f7e50b47ef299 started using quicklists for freeing page table pages and removed the usage of tlb_remove_page() And looking at quicklist_free() and quicklist_free_page(), on a NUMA platform, this can

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: Can we revert this commit for 2.6.23 and look at this code post 2.6.23? I'll happily revert it, but I want to understand this better, so more of an explanation of the codepath that actually does something wrong, please. The quicklists collect

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Linus Torvalds wrote: On Fri, 21 Sep 2007, Christoph Lameter wrote: The quicklists collect pages while we gather pages for TLB flushing. These pages must be kept until the actual TLB flush has occurred. The optimization of releasing off node pages early is

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: On Fri, 21 Sep 2007, Linus Torvalds wrote: On Fri, 21 Sep 2007, Christoph Lameter wrote: The quicklists collect pages while we gather pages for TLB flushing. These pages must be kept until the actual TLB flush has occurred. The

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: The final flush does only check if there are too many pages on the quicklists. Otherwise the quicklist is not dumped/freed (unlike the mmu_gather list) but kept for the following page table page allocations since we have cache hot cpu objects

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: Yeah, and the whole thing seems totally bogus. It totally depends on mmu_gather doing everything right (which very much includes the dependency on mmu gathering disabling preempt). The quicklists have been existing for a long time in this

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Christoph Lameter wrote: The only way to fix it would be to integrate the quicklist stuff *with* the mmu_gather stuff, so that these kinds of implementation issues are explicitly shown in the relationship, instead of havign two independent pieces of code where one

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Linus Torvalds
On Fri, 21 Sep 2007, Linus Torvalds wrote: So for example, if you actually want to use quicklists in the generic TLB-gather implementation, you should replace the struct page *pages[FREE_PTE_NR]; entry in the struct mmu_gather with a set of quicklists instead. Side note: this

Re: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Linus Torvalds wrote: The problem with that commit that I just reverted was that it mixed the two, but not completely. It still kept them separate. The quicklists have a long history and this bug has therefore also been in IA64 for a long time and it also likely exists

RE: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Luck, Tony
The quicklists have a long history and this bug has therefore also been in IA64 for a long time and it also likely exists on sparc64, sh and sh64. We need the patch that I posted to fix the other platforms. And with this fix there would be nothing amiss on x86_64 either. IA64 doesn't do a

RE: x86_64: potential critical issue with quicklists and page table pages

2007-09-21 Thread Christoph Lameter
On Fri, 21 Sep 2007, Luck, Tony wrote: The quicklists have a long history and this bug has therefore also been in IA64 for a long time and it also likely exists on sparc64, sh and sh64. We need the patch that I posted to fix the other platforms. And with this fix there would be nothing