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

[Olaf Hering <olaf@xxxxxxxxx>]

On Mon, Feb 07, George Dunlap wrote:

> 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.

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


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