Re: x86_64: potential critical issue with quicklists and page tablepages

[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 
> > > optimization of releasing off node pages early is therefore not valid.
> > 
> > That should be the "mmu_gather" structure, not the quicklists.
> 
> Oh, and I see what's wrong: you not only switched "free_page()" to 
> "quicklist_free()", you *also* switched "tlb_remove_page()" to 
> "quicklist_free()".
> 
> Ok, that commit is totally and utterly broken. Will revert.

On x86_64 the quicklists partially replace the role of the mmu_gather
list because we would otherwise create more overhead by having to move
pages between the mmu gather and the quicklists lists.
lists.

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 there.

The only problem is that we cannot release any pages before the TLB flush 
has happened. The optimization of releasing off node pages in order to 
keep only node local pages is therefore not valid.



---
 include/linux/quicklist.h |    8 --------
 1 file changed, 8 deletions(-)

Index: linux-2.6/include/linux/quicklist.h
===================================================================
--- linux-2.6.orig/include/linux/quicklist.h	2007-09-21 11:46:44.000000000 -0700
+++ linux-2.6/include/linux/quicklist.h	2007-09-21 11:55:01.000000000 -0700
@@ -56,14 +56,6 @@ static inline void __quicklist_free(int 
 	struct page *page)
 {
 	struct quicklist *q;
-	int nid = page_to_nid(page);
-
-	if (unlikely(nid != numa_node_id())) {
-		if (dtor)
-			dtor(p);
-		__free_page(page);
-		return;
-	}
 
 	q = &get_cpu_var(quicklist)[nr];
 	*(void **)p = q->page;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/