Re: [PATCH 6/9] clockpro-clockpro.patch

[Peter Zijlstra <peter@programming.kicks-ass.net>]

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?

Kind regards,

Peter Zijlstra


> I wonder if this divergence is intended or not, but the attached patch
> gave a major improvement.
> Also, I think there's no reason to call __select_list_hand in those
> functions.
> 
>  n   unpatched   patched
> ========================
> 100   392283     595229
> 200   393087	 584397
> 300   309595	 521259
> 400   283340	 400243
> 500   132298	 303189
> 
> --- linux-2.6.16-rc1-git3-clockpro/mm/clockpro.c.save	2006-02-08 17:20:40.000000000 +0900
> +++ linux-2.6.16-rc1-git3-clockpro/mm/clockpro.c	2006-02-08 17:21:16.000000000 +0900
> @@ -446,7 +446,10 @@ void page_replace_reinsert(struct list_h
>  			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)
> @@ -520,7 +523,10 @@ void page_replace_reinsert_zone(struct z
>  			++dct;
>  		}
>  
> -		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);
>  	}
>  	__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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>