Re: [Xen-devel] [PATCH] x86: fix domain cleanup

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> "Jan Beulich" <jbeulich@xxxxxxxxxx> 27.10.08 12:36 >>>
>The preemptable page type handling changes modified free_page_type()
>behavior without adjusting the call site in relinquish_memory(): Any
>type reference left pending when leaving hypercall handlers is
>associated with a page reference, and when successful free_page_type()
>decrements the type refcount - hence relinquish_memory() must now also
>drop the page reference.

After some more thinking, especially in the context of another bug (see
below), I'm not certain anymore this is the right thing to do, even though
the explanation continues to seem plausible to me. One part of my concern
is that, when the issue this patch attempted to fix is seen, the code path
gets entered only because of DOMAIN_DESTRUCT_AVOID_RECURSION, not
because of either of the conditions mentioned in the preceding comment.
Hence I'm worried that especially for circular 'linear page table' references
this may be wrong (but I don't really know how to properly distinguish this
case from the DOMAIN_DESTRUCT_AVOID_RECURSION leftovers).

As to the PGT_partial case - with the current way this works, the
put_page() seems to be wrong in any case. However, there's a second
(independent) bug in that it currently is possible during domain cleanup to
encounter pages with PGT_partial set, their type reference count being 1,
but their page reference count being 0. This is certainly wrong (it triggers
the BUG_ON() in free_domheap_pages()). As PGT_partial is being handled
so that the type refcount is left at 1, I think it must imply to also keep the
respective page reference. Doing this at the call sites (i.e. be suppressing
the put_page()) seems cumbersome, though, so I'm rather considering
obtaining an extra reference when PGT_partial gets set, and dropping it
when PGT_partial gets cleared. With that, the put_page() mentioned
above would be correct again (as PGT_partial *is* being cleared there).

Btw., the other part of that patch (i.e. the changes to xen/arch/x86/mm.c)
should probably also go into the 3.3 tree.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel