Re: [Xen-devel] [PATCH] x86/mm: print domain IDs instead of pointers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.06.15 at 12:53, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/06/15 11:27, Jan Beulich wrote:
>> Printing pointers to struct domain isn't really useful for initial
>> problem analysis. In get_page() also drop the page only after issuing
>> the log message, so that at the time of printing the state can be
>> considered reasonably consistent.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> There are a few areas of code which seem overzealous with printing Xen
> pointers.
> 
>> ---
>> It also slightly concerns me that page_get_owner_and_reference() can
>> (at least theoretically) return NULL even after having taken a page
>> reference (when ->v.inuse._domain is clear), in which case that
>> reference would never get dropped again. One case where this might be a
>> problem is sh_set_allocation(), which - other than all other uses of
>> page_set_owner(, NULL) - doesn't zap the refcount.
> 
> Looks as if page_get_owner_and_reference() should drop the ref itself if
> ->v.inuse._domain is clear.
> 
> Alternatively, the implication is that page_get_owner_and_reference() is
> called on a page with a real owner, which might justify an ASSERT/BUG.

I think there ought to be an ASSERT(). Unless Keir or Tim know of a
reason why not.

> A different area which concerns me is __acquire_grant_for_copy() which
> ignores the return value.  The caller of __acquire_grant_for_copy() cant
> actually be certain that a ref was taken on the page, based on a return
> value of GNTST_okay.

This is in a path where an in-use active entry was found, i.e. the
page ought to have a non-zero refcount and be owned by the
granting domain (or maybe another one for transitive grants, or
dom_io if this is an MMIO page - I don't recall whether we allow
this or only meant to at some point). I.e. another ASSERT() (or
even BUG()) perhaps.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel