From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Thu, 09 Feb 2006 19:07:56 +0900 From: IWAMOTO Toshihiro Subject: Re: [PATCH 6/9] clockpro-clockpro.patch In-Reply-To: <20060209072240.351A174031@sv1.valinux.co.jp> References: <20051230223952.765.21096.sendpatchset@twins.localnet> <20051230224312.765.58575.sendpatchset@twins.localnet> <20051231002417.GA4913@dmt.cnet> <1136028546.17853.69.camel@twins> <20060105094722.897C574030@sv1.valinux.co.jp> <20060106090135.3525D74031@sv1.valinux.co.jp> <20060124063010.B85C77402D@sv1.valinux.co.jp> <20060124072503.BAF6A7402F@sv1.valinux.co.jp> <1138958705.5450.9.camel@localhost.localdomain> <20060208100505.247D874034@sv1.valinux.co.jp> <1139428810.4668.20.camel@localhost.localdomain> <20060209072240.351A174031@sv1.valinux.co.jp> MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Message-Id: <20060209100756.94E247402F@sv1.valinux.co.jp> Sender: owner-linux-mm@kvack.org Return-Path: To: Peter Zijlstra Cc: IWAMOTO Toshihiro , Rik van Riel , Peter Zijlstra , Marcelo Tosatti , linux-mm@kvack.org, Andrew Morton , Christoph Lameter , Wu Fengguang , Nick Piggin , Marijn Meijles List-ID: At Thu, 09 Feb 2006 16:22:40 +0900, IWAMOTO Toshihiro wrote: > > At Wed, 08 Feb 2006 21:00:10 +0100, > Peter Zijlstra wrote: > > > > On Wed, 2006-02-08 at 19:05 +0900, IWAMOTO Toshihiro wrote: > > > I've noticed the way page_replace_reinsert puts pages to a list is > > > different from what the paper (TR-05-3.pdf) says. > > > > I reread the paper, and you seem to have found a detail I had not > > noticed before. Thanks! > > > > It concerns section 4.3 paragraph 2 and the use of list head as defined > > in section 4.2 paragraph 2. > > > > The way I read it is that pages that have their reference bit set are > > moved to the list head, which according to 4.2.2 is the page right in > > front of hand hot (which would correspond to the tail of the HAND_HOT > > list). > > > > So I agree the current code is wrong. However I read it differently. > > > > In your patch you move all Hot pages to the head of the hot list, which > > effectively leaves them in place. The other pages are moved to the head > > of the cold list, which is right behind hand hot. > > > > So I don't see your condition nor the placement. Could you explain your > > reasoning? > > My patch should be okay for pages which are already hot when Hcold > passes, but I found that the paper says pages which are turned hot by > Hcold should be placed at the list head, which requires a page flag to > implement. I think the PG_test bit could be abused for this purpose. How about this patch? I got a similar benchmark result as before (but probably with statistically significant differences for some n). I found the usage of page_replace_reinsert is quite different and that part of the patch might need to be dropped (but left as is for now). And it seems that __select_list_hand in __page_replace_add need to be removed, too. --- a/mm/clockpro.c.orig 2006-02-06 14:41:40.000000000 +0900 +++ a/mmclockpro.c 2006-02-09 18:52:36.000000000 +0900 @@ -444,9 +444,11 @@ void page_replace_reinsert(struct list_h spin_unlock_irq(&zone->lru_lock); zone = pagezone; spin_lock_irq(&zone->lru_lock); - __select_list_hand(zone, &zone->policy.list_hand[HAND_HOT]); } - list_move(&page->lru, &zone->policy.list_hand[HAND_HOT]); + if (!PageHot(page)) + list_move(&page->lru, &zone->policy.list_hand[HAND_COLD]); + else + list_move(&page->lru, &zone->policy.list_hand[HAND_HOT]); __page_release(zone, page, &pvec); } if (zone) @@ -507,20 +509,25 @@ void page_replace_reinsert_zone(struct z { struct pagevec pvec; unsigned long dct = 0; + int newhot; pagevec_init(&pvec, 1); spin_lock_irq(&zone->lru_lock); - __select_list_hand(zone, &zone->policy.list_hand[HAND_HOT]); while (!list_empty(page_list)) { struct page *page = lru_to_page(page_list); prefetchw_prev_lru_page(page, page_list, flags); if (PageHot(page) && PageTest(page)) { ClearPageTest(page); + newhot = 1; ++dct; - } + } else + newhot = 0; - list_move(&page->lru, &zone->policy.list_hand[HAND_HOT]); + if (!PageHot(page) || newhot) + list_move(&page->lru, &zone->policy.list_hand[HAND_COLD]); + else + list_move(&page->lru, &zone->policy.list_hand[HAND_HOT]); __page_release(zone, page, &pvec); } __cold_target_inc(zone, dct); -- 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