Re: [patch 1/2] x86: track memtype for RAM in page struct - v3

[Nick Piggin <nickpiggin@yahoo.com.au>]

On Wednesday 01 October 2008 07:14, Suresh Siddha wrote:
> On Tue, Sep 30, 2008 at 04:21:28AM -0700, Ingo Molnar wrote:
> > * Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> > > On Thursday 25 September 2008 01:53, Venki Pallipadi wrote:
> > > >  /*
> > > > + * RED-PEN:  TODO: Add PageReserved() check as well here,
> > > > + * once we add SetPageReserved() to all the drivers using
> > > > + * set_memory_* or set_pages_*.
> > > > + *
> > > > + * This will help prevent accidentally freeing pages
> > > > + * before setting the attribute back to WB.
> > > > + */
> > >
> > > I'd rather we didn't add any more uses of PageReserved without a
> > > really good reason.
> > >
> > > At this point in time (or at least last time I looked, a year or
> > > two ago), it isn't a whole lot of work to remove PG_reserved
> > > completely. If the waters get muddied up again, it could require
> > > another significant rework to remove it in future...
> >
> > agreed.
>
> If a driver by mistake free's a RAM page before changing its memory
> attribute back to WB, we want the generic -mm to catch it.
>
> Today, free_pages_check() prevents freeing the page with PageReserved set.

That's gone away in -mm.


> We want to use this and make sure that either set_page_uc/wc() or the
> driver calling these API's set the PageReserved bit. There are already some
> drivers which do SetPageReserved() before changing the attribute.
>
> I don't know the history behind PageReserved. But is there a recommended
> way to achieve what we want? Either we need to use PageReserved bit or  add
> some arch specific checks (in the x86 case, check arch_1) in
> free_pages_check(). Right?

It would be better if another flag is used, yes.

Setting PageReserved used to prevent put_page from decrementing the page's
refcount. Most drivers still do it just because we kept that code in there
to catch any bugs caused by the removal of PageReserved check from the
refcounting.