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

["Jan Beulich" <JBeulich@novell.com>]

>>> On 22.03.11 at 20:21, Olaf Hering <olaf@aepfle.de> wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1300813685 -3600
> # Node ID 08df96398bff82a8924a37eda6ddffd1ada3f407
> # Parent  c81f0ef5a77d90fbf108d3efe489d08df45b63c2
> xentrace: fix t_info_pages calculation for the default case
> 
> The default tracebuffer size of 32 pages was not tested with the previous 
> patch.
> As a result, t_info_pages will become zero and alloc_xenheap_pages() fails.
> Catch this case and allocate at least one page.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r c81f0ef5a77d -r 08df96398bff xen/common/trace.c
> --- a/xen/common/trace.c	Mon Mar 21 14:52:27 2011 +0000
> +++ b/xen/common/trace.c	Tue Mar 22 18:08:05 2011 +0100
> @@ -125,7 +125,7 @@
>      t_info_pages = num_online_cpus() * pages + t_info_first_offset;
>      t_info_pages *= sizeof(uint32_t);
>      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?

Jan

>          t_info_pages++;
>      return pages;
>  }