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

Peter Zijlstra
2006-02-09T06:57:52Z

On Wed, 2006-02-08 at 21:00 +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).

This just plain doesn't make sense though.

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

not intended

> > Also, I think there's no reason to call __select_list_hand in those
> > functions.

/me *nods*