Re: [PATCH 17/33] readahead: context based method

Wu Fengguang
2006-05-24T13:33:58Z

On Wed, May 24, 2006 at 02:37:48PM +0200, Peter Zijlstra wrote:
> On Wed, 2006-05-24 at 19:13 +0800, Wu Fengguang wrote:
>
> > +#define PAGE_REFCNT_0 0
> > +#define PAGE_REFCNT_1 (1 << PG_referenced)
> > +#define PAGE_REFCNT_2 (1 << PG_active)
> > +#define PAGE_REFCNT_3 ((1 << PG_active) | (1 << PG_referenced))
> > +#define PAGE_REFCNT_MASK PAGE_REFCNT_3
> > +
> > +/*
> > + * STATUS REFERENCE COUNT
> > + * __ 0
> > + * _R PAGE_REFCNT_1
> > + * A_ PAGE_REFCNT_2
> > + * AR PAGE_REFCNT_3
> > + *
> > + * A/R: Active / Referenced
> > + */
> > +static inline unsigned long page_refcnt(struct page *page)
> > +{
> > + return page->flags & PAGE_REFCNT_MASK;
> > +}
> > +
> > +/*
> > + * STATUS REFERENCE COUNT TYPE
> > + * __ 0 fresh
> > + * _R PAGE_REFCNT_1 stale
> > + * A_ PAGE_REFCNT_2 disturbed once
> > + * AR PAGE_REFCNT_3 disturbed twice
> > + *
> > + * A/R: Active / Referenced
> > + */
> > +static inline unsigned long cold_page_refcnt(struct page *page)
> > +{
> > + if (!page || PageActive(page))
> > + return 0;
> > +
> > + return page_refcnt(page);
> > +}
> > +
>
> Why all of this if all you're ever going to use is cold_page_refcnt.

Well, the two functions have a long history...

There has been a PG_activate which makes the two functions quite
different. It was later removed for fear of the behavior changes it
introduced. However, there's still possibility that someone
reintroduce similar flags in the future :)

> What about something like this:
>
> static inline int cold_page_referenced(struct page *page)
> {
> if (!page || PageActive(page))
> return 0;
> return !!PageReferenced(page);
> }

Ah, here's another theory: the algorithm uses reference count
conceptually, so it may be better to retain the current form.

Thanks,
Wu