Re: [Xen-devel] [PATCH] xentrace: dynamic tracebuffer size allocation

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Sun, Feb 6, 2011 at 1:39 PM, Olaf Hering <olaf@xxxxxxxxx> wrote:
> @@ -85,20 +100,30 @@ static void calc_tinfo_first_offset(void
>  }
>
>  /**
> - * check_tbuf_size - check to make sure that the proposed size will fit
> + * calculate_tbuf_size - check to make sure that the proposed size will fit
>  * in the currently sized struct t_info and allows prod and cons to
>  * reach double the value without overflow.
>  */
> -static int check_tbuf_size(u32 pages)
> +static int calculate_tbuf_size(unsigned int pages)
>  {
>     struct t_buf dummy;
> -    typeof(dummy.prod) size;
> -
> -    size = ((typeof(dummy.prod))pages)  * PAGE_SIZE;
> -
> -    return (size / PAGE_SIZE != pages)
> -           || (size + size < size)
> -           || (num_online_cpus() * pages + t_info_first_offset > T_INFO_SIZE 
> / sizeof(uint32_t));
> +    typeof(dummy.prod) size = -1;
> +
> +    /* max size holds up to n pages */
> +    size /= PAGE_SIZE;

size=-1, then size /= PAGE_SIZE?  Is this a clever way of finding the
maximum buffer size able to be pointed to?  If so, it needs a comment
explaining why it works; I'm not convinced just by looking at it this
is will work properly.

Other than that, it looks good:

Regarding the size fo the patch: a lot of it is just shuffling code
around.  xentrace isn't really on the critical path of functionality
either.  But overall, I think we're meant to have had a feature
freeze, and this certainly isn't a bug fix.

 -George

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