[ltt-dev] Force buffer sizes to be at least page size

[Mathieu Desnoyers]

* Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> * Mathieu Desnoyers (compudj at krystal.dyndns.org) wrote:
> > * Chase Douglas (cndougla at linux.vnet.ibm.com) wrote:
> > > During some testing across various architectures we found that our ppc64 
> > > environment with 64K page sizes was causing lttctl to hang when  
> > > destroying a trace if the buffer size was less than 64K  
> > > (channel.all.bufsize). We had been testing with 16K buffer sizes, and  
> > > every other architecture ran fine. This led us to believe that ltt is  
> > > not handling buffer sizes less than a page size correctly.
> > >
> > > We did not determine the root cause of the failure, instead opting for a 
> > > quick patch to force the buffer sizes to be at least page size. I have 
> > > attached the patch. Please take this under advisement either to  
> > > determine the root cause or to force proper buffer sizes as necessary.
> > >
> > 
> > Actually, the subbuffer size (not only the buffer size) should be at
> > least a page. I'll do a patch for LTTng shortly.
> > 
> > Thanks for the report !
> > 
> 
> Can you give this patch a try to see if it fixes your problem ? (without
> your lttctl patch, the problem must be dealt at the kernel-level)
> 

The patch seems to work OK on my system with 4k pages when trying
forcing small subbuffer size values with lttctl.

It's now in lttng 0.148.

Feedback is still welcome,

Thanks,

Mathieu

> 
> LTTng check min subbuffer size page size
> 
> The mininum size of a subbuffer (not just buffer) must be at least a page.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at polymtl.ca>
> ---
>  include/linux/ltt-relay.h |    3 ---
>  ltt/ltt-relay-alloc.c     |   16 ++++++++++++----
>  ltt/ltt-tracer.c          |    4 ++++
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6-lttng/ltt/ltt-relay-alloc.c
> ===================================================================
> --- linux-2.6-lttng.orig/ltt/ltt-relay-alloc.c	2009-07-15 13:10:29.000000000 -0400
> +++ linux-2.6-lttng/ltt/ltt-relay-alloc.c	2009-07-15 13:26:15.000000000 -0400
> @@ -39,7 +39,6 @@ static int relay_alloc_buf(struct rchan_
>  	struct page **pages;
>  	void **virt;
>  
> -	*size = PAGE_ALIGN(*size);
>  	n_pages = *size >> PAGE_SHIFT;
>  	n_pages_per_sb = n_pages >> get_count_order(n_subbufs);
>  	if (extra_reader_sb)
> @@ -421,8 +420,8 @@ static int __cpuinit relay_hotcpu_callba
>   *	ltt_relay_open - create a new relay channel
>   *	@base_filename: base name of files to create
>   *	@parent: dentry of parent directory, %NULL for root directory
> - *	@subbuf_size: size of sub-buffers
> - *	@n_subbufs: number of sub-buffers
> + *	@subbuf_size: size of sub-buffers (> PAGE_SIZE, power of 2)
> + *	@n_subbufs: number of sub-buffers (power of 2)
>   *	@cb: client callback functions
>   *	@private_data: user-defined data
>   *	@extra_reader_sb: allocate an extra subbuffer for the reader
> @@ -454,11 +453,20 @@ struct rchan *ltt_relay_open(const char 
>  	if (!chan)
>  		return NULL;
>  
> +	/* Check that the subbuffer size is larger than a page. */
> +	WARN_ON_ONCE(subbuf_size < PAGE_SIZE);
> +
> +	/*
> +	 * Make sure the number of subbuffers and subbuffer size are power of 2.
> +	 */
> +	WARN_ON_ONCE(hweight32(subbuf_size) != 1);
> +	WARN_ON(hweight32(n_subbufs) != 1);
> +
>  	chan->version = LTT_RELAY_CHANNEL_VERSION;
>  	chan->n_subbufs = n_subbufs;
>  	chan->subbuf_size = subbuf_size;
>  	chan->subbuf_size_order = get_count_order(subbuf_size);
> -	chan->alloc_size = FIX_SIZE(subbuf_size * n_subbufs);
> +	chan->alloc_size = subbuf_size * n_subbufs;
>  	chan->parent = parent;
>  	chan->private_data = private_data;
>  	chan->extra_reader_sb = extra_reader_sb;
> Index: linux-2.6-lttng/include/linux/ltt-relay.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/linux/ltt-relay.h	2009-07-15 13:16:51.000000000 -0400
> +++ linux-2.6-lttng/include/linux/ltt-relay.h	2009-07-15 13:16:59.000000000 -0400
> @@ -22,9 +22,6 @@
>  #include <linux/mm.h>
>  #include <linux/ltt-core.h>
>  
> -/* Needs a _much_ better name... */
> -#define FIX_SIZE(x) ((((x) - 1) & PAGE_MASK) + PAGE_SIZE)
> -
>  /* Use lowest pointer bit to show the sub-buffer has no reference. */
>  #define RCHAN_NOREF_FLAG	0x1UL
>  
> Index: linux-2.6-lttng/ltt/ltt-tracer.c
> ===================================================================
> --- linux-2.6-lttng.orig/ltt/ltt-tracer.c	2009-07-15 13:21:09.000000000 -0400
> +++ linux-2.6-lttng/ltt/ltt-tracer.c	2009-07-15 13:25:08.000000000 -0400
> @@ -510,6 +510,10 @@ EXPORT_SYMBOL_GPL(ltt_release_trace);
>  static inline void prepare_chan_size_num(unsigned int *subbuf_size,
>  					 unsigned int *n_subbufs)
>  {
> +	/* Make sure the subbuffer size is larger than a page */
> +	*subbuf_size = max_t(unsigned int, *subbuf_size, PAGE_SIZE);
> +
> +	/* round to next power of 2 */
>  	*subbuf_size = 1 << get_count_order(*subbuf_size);
>  	*n_subbufs = 1 << get_count_order(*n_subbufs);
>  
> 
> -- 
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68