From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/mm: print domain IDs instead of pointers Date: Fri, 5 Jun 2015 11:53:31 +0100 Message-ID: <55717FAB.20309@citrix.com> References: <557195AE02000078000813DC@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z0pFs-0000rz-RK for xen-devel@lists.xenproject.org; Fri, 05 Jun 2015 10:53:40 +0000 In-Reply-To: <557195AE02000078000813DC@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Tim Deegan , Keir Fraser List-Id: xen-devel@lists.xenproject.org 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 Reviewed-by: Andrew Cooper 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. 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. ~Andrew