need for ClearPageReserved() in blktap2 code

need for ClearPageReserved() in blktap2 code

[Jan Beulich]

Jake, Dutch,

while I can see the need for this on the ring pages, I have some
difficulty understanding why it needs to be invoked on the individual
data pages as they get removed from ring->foreign_map.map: I
understand that SetPageReserved() when inserting them is needed
(or at least desirable) since in the 2.6.18 and pv-ops trees the
balloon driver doesn't guarantee pages being returned from
alloc_empty_pages_and_page_vec() are marked reserved (which
minimally I consider an inconsistency). In our trees, the balloon
driver does guarantee this, but it expects the pages to also be
returned in reserved state - which blktap2 breaks because of
the arbitrary(?) clearing of the flag.

The question hence really is whether that clearing is really
necessary, i.e. whether the pages, when not in some
ring->foreign_map.map[], validly and reasonably can (should) be
considered non-reserved - to me it appears there's no need for
this.

Thanks for letting my know your thoughts on this,
Jan

RE: need for ClearPageReserved() in blktap2 code

[Jake Wires]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@novell.com]
> Sent: Monday, August 09, 2010 8:47 AM
> To: Jake Wires; dmeyer@cs.ubc.ca
> Cc: xen-devel@lists.xensource.com
> Subject: need for ClearPageReserved() in blktap2 code
> 
> Jake, Dutch,
> 
> while I can see the need for this on the ring pages, I have some
> difficulty understanding why it needs to be invoked on the individual
> data pages as they get removed from ring->foreign_map.map: I
> understand that SetPageReserved() when inserting them is needed
> (or at least desirable) since in the 2.6.18 and pv-ops trees the
> balloon driver doesn't guarantee pages being returned from
> alloc_empty_pages_and_page_vec() are marked reserved (which
> minimally I consider an inconsistency). In our trees, the balloon
> driver does guarantee this, but it expects the pages to also be
> returned in reserved state - which blktap2 breaks because of
> the arbitrary(?) clearing of the flag.
> 
> The question hence really is whether that clearing is really
> necessary, i.e. whether the pages, when not in some
> ring->foreign_map.map[], validly and reasonably can (should) be
> considered non-reserved - to me it appears there's no need for
> this.

Yup, seems reasonable for these pages to remain reserved the whole time. 

> 
> Thanks for letting my know your thoughts on this,
> Jan