Re: [RFC][patch 0/2] mm: remove PageReserved

Daniel Phillips
2005-08-08T21:09:59Z

On Sunday 07 August 2005 13:28, Nick Piggin wrote:
> Hi,
>
> I'll be looking to send these off to Andrew after 2.6.14 opens,
> with the aim of having them merged by 2.6.15 hopefully.
>
> It doesn't look like they'll be able to easily free up a page
> flag for 2 reasons. First, PageReserved will probably be kept
> around for at least one release. Second, swsusp and some arch
> code (ioremap) wants to know about struct pages that don't point
> to valid RAM - currently they use PageReserved, but we'll probably
> just introduce a PageValidRAM or something when PageReserved goes.
>
> I believe this makes memory management cleaner and easier to
> understand.

Agreed, I've always looked askance at that particular page flag. (Suggestion
for your next act:

> My other reason behind this is that the lockless
> pagecache patches needs it for sane page refcounting.
>
> If anyone has an issue with the patches or my merge plan, let's
> get some discussion going.

You forgot to mention what replaces PageReserved: the VM_RESERVED vma flag,
which is now added to the whole zap_pte call chain. A slight efficiency win?
Anyway, it looks like forward progress because some inner loops are a little
straighter. I've always wondered what PG_reserved was actually doing, and
now I know: compensating for the missing vma parameter in the zap call
chains.

Why don't you pass the vma in zap_details? For that matter, why are addr and
end still passed down the zap chain when zap_details appears to duplicate
that information? OK, it is because zap_details is NULL in about twice as
many places as it carries data. But since the details parameter is already
there, would it not make sense to press it into service to slim down those
parameter lists a little?

What stops swsusp from also using the vma flag? Why does swsusp need both
PG_reserved and PG_nosave?

Is there automated testing planned for this one? It looks right as closely as
I've read, but it tickles an awful lot of code.

Regards,

Daniel