From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH 1 of 5] xentrace: fix t_info_pages calculation for the default case Date: Wed, 23 Mar 2011 13:46:54 +0100 Message-ID: <20110323124654.GA4696@aepfle.de> References: <20110323112040.GB28535@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel@lists.xensource.com, George Dunlap , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Wed, Mar 23, Keir Fraser wrote: > On 23/03/2011 11:20, "Olaf Hering" 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). > > > > I did not know about PFN_UP() until now, using it would work as well. > > As opposed to the existing code (even including your latest patch) which > doesn't work properly. You need to respin at least your patch 1/5. I will send the bugfix now. Olaf