From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC 01/19] xen/arm: guest_physmap_remove_page: Print a warning if we fail to unmap the page Date: Thu, 03 Jul 2014 12:17:58 +0100 Message-ID: <53B53BE6.4000803@linaro.org> References: <1402935486-29136-1-git-send-email-julien.grall@linaro.org> <1402935486-29136-2-git-send-email-julien.grall@linaro.org> <1404384738.14865.67.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X2f1i-0007v6-Ep for xen-devel@lists.xenproject.org; Thu, 03 Jul 2014 11:18:06 +0000 Received: by mail-wg0-f52.google.com with SMTP id b13so62624wgh.11 for ; Thu, 03 Jul 2014 04:18:04 -0700 (PDT) In-Reply-To: <1404384738.14865.67.camel@kazak.uk.xensource.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, stefano.stabellini@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org Hi Ian, On 07/03/2014 11:52 AM, Ian Campbell wrote: > On Mon, 2014-06-16 at 17:17 +0100, Julien Grall wrote: >> The function guest_physmap_remove_page does't have a return value. With >> the change "arch/arm: add consistency check to REMOVE p2m changes", >> apply_p2m_changes can unlikely fail. > > Looking at v9 of that patch I don't see it adding any new failures, is > this comment (I suppose written against an older version) still > accurate? It doesn't look like accurate for v9. I can change the commit message to explain that apply_p2m_changes may fail. So it's better to log it/ >> Warn the user in this case. > > Given that apply_p2m_changes can fail it seems reasonable to log > regardless of the above: > Acked-by: Ian Campbell > Two questions: > > Would it be better to instead ensure that apply_p2m_changes always logs > on failure? I suppose it would be more able to give a specific message. I didn't though about this solution. It may be easier for debugging later. > On failure do we retain any reference counts to prevent these pages > getting reused? We don't have any mapping reference count so it's fine. I've asked few changes in Arianna's series to clear the p2m even if the mapping doesn't correspond. Otherwise we may have issue with foreign mapping. I will drop this patch from the series and send a separate patch. Regards, -- Julien Grall