From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 1 of 5] xentrace: fix t_info_pages calculation for the default case Date: Wed, 23 Mar 2011 10:22:16 +0000 Message-ID: References: <4D89D5920200007800037DBE@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D89D5920200007800037DBE@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , Olaf Hering Cc: xen-devel@lists.xensource.com, George Dunlap List-Id: xen-devel@lists.xenproject.org On 23/03/2011 10:12, "Jan Beulich" wrote: >> t_info_pages /= PAGE_SIZE; >> - if ( t_info_pages % PAGE_SIZE ) >> + if ( t_info_pages % PAGE_SIZE || t_info_pages == 0 ) > > While certainly not having a significant effect, to the unsuspecting > reader this looks like a bug - is it really meant to be a remainder > operation on the *result* of a division (rather than on the original > dividend)? Couldn't you just (ab)use PFN_UP() here? By which you mean to replace the division and subsequent if statement with t_info_pages = PFN_UP(t_info_pages). That would make sense to me. The suggested change in the patch series looks nonsensical. -- Keir