From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: Re: [PATCH 01/10] xen: Make all reserved pages for the balloon be INVALID_P2M_ENTRY. Date: Wed, 22 Dec 2010 08:47:10 +0000 Message-ID: <1293007630.3998.36.camel@localhost.localdomain> References: <1292967460-15709-1-git-send-email-konrad.wilk@oracle.com> <1292967460-15709-2-git-send-email-konrad.wilk@oracle.com> <4D1127FC.8060102@goop.org> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D1127FC.8060102@goop.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jeremy Fitzhardinge Cc: "xen-devel@lists.xensource.com" , Konrad Rzeszutek Wilk , "linux-kernel@vger.kernel.org" , Jan Beulich , Konrad Rzeszutek Wilk , "hpa@zytor.com" List-Id: xen-devel@lists.xenproject.org On Tue, 2010-12-21 at 22:19 +0000, Jeremy Fitzhardinge wrote: > On 12/21/2010 01:37 PM, Konrad Rzeszutek Wilk wrote: > > + BUG_ON(!set_phys_to_machine(pfn, INVALID_P2M_ENTRY)); [...] > I'm not a fan of hiding real side-effectful code in a BUG_ON > predicate. > > > + BUG_ON(get_phys_to_machine(pfn) != INVALID_P2M_ENTRY); I'm also not a fan of this "is the previous I called function buggy" BUG_ON. If we aren't confident that set_phys_to_machine() is doing the right thing then adding a post-condition check to that function would be a more preferable option. Ian.