From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keir Fraser <keir@xen.org>
Subject: Re: [PATCH 1 of 5] xentrace: fix t_info_pages calculation
	for the default case
Date: Wed, 23 Mar 2011 12:33:03 +0000
Message-ID: <C9AF98FF.2BC96%keir@xen.org>
References: <20110323112040.GB28535@aepfle.de>
Mime-Version: 1.0
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <20110323112040.GB28535@aepfle.de>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Olaf Hering <olaf@aepfle.de>
Cc: xen-devel@lists.xensource.com, George Dunlap <george.dunlap@citrix.com>, Jan Beulich <JBeulich@novell.com>
List-Id: xen-devel@lists.xenproject.org

On 23/03/2011 11:20, "Olaf Hering" <olaf@aepfle.de> 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.

 -- Keir