From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH] xentrace: dynamic tracebuffer size allocation Date: Mon, 7 Feb 2011 18:55:12 +0100 Message-ID: <20110207175512.GA13123@aepfle.de> References: <20110205140717.GA3224@aepfle.de> <20110206133913.GA7487@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: George Dunlap Cc: xen-devel@lists.xensource.com, Keir Fraser List-Id: xen-devel@lists.xenproject.org On Mon, Feb 07, George Dunlap wrote: > On Sun, Feb 6, 2011 at 1:39 PM, Olaf Hering wrote: > > @@ -85,20 +100,30 @@ static void calc_tinfo_first_offset(void > > =C2=A0} > > > > =C2=A0/** > > - * check_tbuf_size - check to make sure that the proposed size will = fit > > + * calculate_tbuf_size - check to make sure that the proposed size w= ill fit > > =C2=A0* in the currently sized struct t_info and allows prod and cons= to > > =C2=A0* reach double the value without overflow. > > =C2=A0*/ > > -static int check_tbuf_size(u32 pages) > > +static int calculate_tbuf_size(unsigned int pages) > > =C2=A0{ > > =C2=A0 =C2=A0 struct t_buf dummy; > > - =C2=A0 =C2=A0typeof(dummy.prod) size; > > - > > - =C2=A0 =C2=A0size =3D ((typeof(dummy.prod))pages) =C2=A0* PAGE_SIZE= ; > > - > > - =C2=A0 =C2=A0return (size / PAGE_SIZE !=3D pages) > > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 || (size + size < size) > > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 || (num_online_cpus() * pages + = t_info_first_offset > T_INFO_SIZE / sizeof(uint32_t)); > > + =C2=A0 =C2=A0typeof(dummy.prod) size =3D -1; > > + > > + =C2=A0 =C2=A0/* max size holds up to n pages */ > > + =C2=A0 =C2=A0size /=3D PAGE_SIZE; >=20 > size=3D-1, then size /=3D PAGE_SIZE? Is this a clever way of finding t= he > 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. This was a head-scratcher for me as well. The typeof() was probably meant to make it independent from changes in t_buf. My version does not cover signed types, and I couldnt come up with a simple way to get to the max value of any given type. So the -1 will cover just the unsigned types. Assuming the index values will never get a signed type, it works. Olaf