[lttng-dev] [PATCH lttng-tools] Fix: change default UST per UID subbuffer size

[Mathieu Desnoyers]

* David Goulet (dgoulet at efficios.com) wrote:
> 
> 
> Mathieu Desnoyers:
> > * David Goulet (dgoulet at efficios.com) wrote:
> >> The default per UID subbuffer size are changed from 4096 bytes to 131072
> >> bytes (128k).
> >>
> >> Fixes #524
> >>
> >> Signed-off-by: David Goulet <dgoulet at efficios.com>
> >> ---
> >>  src/bin/lttng-sessiond/channel.c |   40 +++++++++++++++++++++++++-------------
> >>  src/bin/lttng-sessiond/channel.h |    3 ++-
> >>  src/bin/lttng-sessiond/cmd.c     |   14 ++++++++-----
> >>  src/common/defaults.c            |    9 ++++++---
> >>  src/common/defaults.h            |   25 ++++++++++++++++++------
> >>  src/lib/lttng-ctl/lttng-ctl.c    |    6 +++++-
> >>  6 files changed, 67 insertions(+), 30 deletions(-)
> > [...]
> > 
> >> diff --git a/src/common/defaults.h b/src/common/defaults.h
> >> index fb6a975..4606976 100644
> >> --- a/src/common/defaults.h
> >> +++ b/src/common/defaults.h
> >> @@ -148,7 +148,8 @@
> >>  /* User space defaults */
> >>  
> >>  /* Must be a power of 2 */
> >> -#define DEFAULT_UST_CHANNEL_SUBBUF_SIZE     4096    /* bytes */
> >> +#define DEFAULT_UST_PID_CHANNEL_SUBBUF_SIZE 4096    /* bytes */
> >> +#define DEFAULT_UST_UID_CHANNEL_SUBBUF_SIZE 131072  /* bytes */
> > 
> > Since we introduce DEFAULT_UST_{PID,UID}_* for SUBBUF_SIZE, I think it
> > would make sense to introduce PID and UID defaults for SUBBUF_NUM and
> > OUTPUT. They can simply map to their respective DEFAULT_UST_CHANNEL
> > defaults. I recommend we rename DEFAULT_UST_CHANNEL_* defaults to
> > _DEFAULT_UST_CHANNEL_* (leading underscore) so we ensure we don't forget
> > any site that uses the old macro. The code that uses those should always
> > use the UID/PID macros from now on. Having this in place will make it
> > much easier to make default value changes in the future if need be
> > without the risk of forgetting to changes some instances in the code
> > (see commit 9f778c9a8f1d65f5bfdde7cfd7294492d6fdd34c as an example of a
> > case we had to fix an issue that arised from inconsistency).
> 
> Not sure I understand here. Don't you mean rename DEFAULT_CHANNEL_* with
> an underscore in order to make sure only domain specific values are used
> in the code base? or every UST macros that are not used outside the
> scope of default.h ?

Resolved by phone discussion. I look forward to the updated patch.

Thanks,

Mathieu

> 
> Actually, subbuffer size are accessed through a getter in default.c
> returning the max() value between our defaults and a page size. That
> said, this means their should be *no* call site that uses the subbuffer
> size macro.
> 
> > 
> > Moreover, I think you forgot to update the documentation (manpage +
> > enable-channel --help).
> 
> Right.
> 
> Cheers!
> David
> 
> > 
> > Thanks,
> > 
> > Mathieu
> > 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com