WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Olaf Hering <olaf@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xentrace: dynamic tracebuffer size allocation
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
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=3DBzPQgCzky/8tpeYh4r8tyM8au4LRB7A9qKchaNc0c=; b=BVqPInGAaKl3VcCKaiRNE3d5vbDtlZ4sqnqAb2uWEd2iJ78W3lJt8FCBscS3N0bR2p ScYhiIaoNf9VobhFkCSH+mL/FG5+j8T8XcSKe5W4B2ulkji6iYGR92/rJp25sieIxp4I AL/BooiSVrkdV3AlLntEQ3JbGqdyFSmpbBBUM=
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=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110206133913.GA7487@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20110205140717.GA3224@xxxxxxxxx> <C9736502.12C07%keir@xxxxxxx> <20110206133913.GA7487@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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