Re: [Xen-devel] [PATCH 1 of 5] xentrace: fix t_info_pages calculation for the default case

[Keir Fraser <keir@xxxxxxx>]

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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel