[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Olaf Hering <olaf@xxxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
  • Date: Mon, 7 Feb 2011 17:38:09 +0000
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Keir Fraser <keir@xxxxxxx>
  • Delivery-date: Mon, 07 Feb 2011 09:38:54 -0800
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=nXWlGke3wkHzZc8+BjRE3fvyHzy4GJ8CTH0zaEmVRMK526PvVWdPfAuYFOsdH0XDEz 9W3lzEx0o+Ey9PvCatdV7VPaUsphekPC/53KTcPisioPtMwer0QgIumlthIjhjDqE0wa pCdEK7AejHzaDYTt5miyUZ2udPlwTn2TLcjMU=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.