[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Xen-devel] Re: [PATCH 2 of 3] xentrace: use tbuf_size for overflow check
On Wed, 2011-03-30 at 19:04 +0100, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@xxxxxxxxx>
> # Date 1301424075 -7200
> # Node ID 3e95e737bc51c2295926e4f23389b1cb161d6d7b
> # Parent 8a2ce5e49b2c5f2e013734b5d53eae37572f4101
> xentrace: use tbuf_size for overflow check
> The calculated number of per-cpu trace pages is stored in t_info and
> shared with tools like xentrace. Since its an u16 the value may overflow
> because the current check is based on u32.
Hmm -- while this is true, it's possible this may change in the future.
If we ever changed t_info.tbuf_size to be u32, then t_buf.prod/cons
would again be the limiting factor.
Should we perhaps add both checks?
> Using the u16 means each cpu could in theory use up to 256MB as trace
> buffer. However such a large allocation will currently fail on x86 due
> to the MAX_ORDER limit.
FWIW, I don't believe that there's any reason the allocations have to be
contiguous any more. I kept them contiguous to minimize the changes to
the moving parts near a release. But the new system has been pretty
well tested now, so I think looking at non-contiguous allocations may be
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> diff -r 8a2ce5e49b2c -r 3e95e737bc51 xen/common/trace.c
> --- a/xen/common/trace.c Tue Mar 29 20:37:20 2011 +0200
> +++ b/xen/common/trace.c Tue Mar 29 20:41:15 2011 +0200
> @@ -104,25 +104,26 @@
> * 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.
> + * tbuf_size is u16, while t_buf prod/cons are u32, so using tbuf_size for
> + * overflow check is good.
> + * The t_info layout is fixed and cant be changed without breaking xentrace.
> * Initialize t_info_pages based on number of trace pages.
> static int calculate_tbuf_size(unsigned int pages)
> - struct t_buf dummy;
> - typeof(dummy.prod) size;
> + struct t_info dummy;
> + typeof(dummy.tbuf_size) max_pages;
> unsigned int t_info_words;
> /* force maximum value for an unsigned type */
> - size = -1;
> + max_pages = -1;
> - /* max size holds up to n pages */
> - size /= PAGE_SIZE;
> - if ( pages > size )
> + if ( pages > max_pages )
> printk(XENLOG_INFO "xentrace: requested number of %u pages "
> "reduced to %u\n",
> - pages, (unsigned int)size);
> - pages = size;
> + pages, max_pages);
> + pages = max_pages;
> t_info_words = num_online_cpus() * pages * sizeof(uint32_t);
Xen-devel mailing list