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

[David Goulet]

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 ?

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
>