https://bugs.lttng.org/issues/228
https://bugs.lttng.org/issues/235

The attached patch should do it.


Ettore

On 10/5/2012 4:46 PM, David Goulet wrote:
> Hey,
> 
> We have right now an opened bug about this exact issue. We will soon fix this
> and, as you said, help considerably the user experience with subbuffer size :)
> 
> Thanks for the report!
> 
> Cheers
> David
> 
> On 09/05/12 08:51 PM, Ettore Del Negro wrote:
>> Hello, i have a simple question which may lead to a small user experience
>> improvement:
> 
>>> lttng create tsession
>> Session tsession created. Traces will be written in
>> .../lttng-traces/tsession-20120510-015819
>>> lttng enable-channel tchan -u -s tsession --subbuf-size=1024
>> UST channel tchan enabled for session tsession
> 
>> a message stating that buffer size is set to a value different than default
>> would be nice
> 
>>> lttng enable-channel tchan -u -s tsession --subbuf-size=1022
>> UST channel tchan enabled for session tsession
> 
>> What happens? Is it set to a value not power of 2? Is it set to the default
>> value? (i guess the latter). The help says: --subbuf-size SIZE   Subbuffer
>> size in bytes (default: 4096, kernel default: 262144) Needs to be a power
>> of 2 for kernel and ust tracers
> 
>> Thanks, Ettore
> 
>> _______________________________________________ lttng-dev mailing list 
>> [email protected] 
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 
> 
diff --git a/include/lttng/lttng.h b/include/lttng/lttng.h
index f621fa8..7d83119 100644
--- a/include/lttng/lttng.h
+++ b/include/lttng/lttng.h
@@ -236,7 +236,7 @@ struct lttng_event {
 #define LTTNG_CHANNEL_ATTR_PADDING1        LTTNG_SYMBOL_NAME_LEN + 32
 struct lttng_channel_attr {
        int overwrite;                      /* 1: overwrite, 0: discard */
-       uint64_t subbuf_size;               /* bytes */
+       uint64_t subbuf_size;               /* bytes, power of 2 */
        uint64_t num_subbuf;                /* power of 2 */
        unsigned int switch_timer_interval; /* usec */
        unsigned int read_timer_interval;   /* usec */
diff --git a/src/bin/lttng/commands/enable_channels.c 
b/src/bin/lttng/commands/enable_channels.c
index 9b5f8d6..4d3eb15 100644
--- a/src/bin/lttng/commands/enable_channels.c
+++ b/src/bin/lttng/commands/enable_channels.c
@@ -68,8 +68,8 @@ static struct poptOption long_options[] = {
 #endif
        {"discard",        0,   POPT_ARG_NONE, 0, OPT_DISCARD, 0, 0},
        {"overwrite",      0,   POPT_ARG_NONE, 0, OPT_OVERWRITE, 0, 0},
-       {"subbuf-size",    0,   POPT_ARG_DOUBLE, 0, OPT_SUBBUF_SIZE, 0, 0},
-       {"num-subbuf",     0,   POPT_ARG_INT, 0, OPT_NUM_SUBBUF, 0, 0},
+       {"subbuf-size",    0,   POPT_ARG_STRING, 0, OPT_SUBBUF_SIZE, 0, 0},
+       {"num-subbuf",     0,   POPT_ARG_STRING, 0, OPT_NUM_SUBBUF, 0, 0},
        {"switch-timer",   0,   POPT_ARG_INT, 0, OPT_SWITCH_TIMER, 0, 0},
        {"read-timer",     0,   POPT_ARG_INT, 0, OPT_READ_TIMER, 0, 0},
        {"list-options",   0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, 
NULL},
@@ -106,12 +106,14 @@ static void usage(FILE *ofp)
                DEFAULT_CHANNEL_SUBBUF_SIZE,
                DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE);
        fprintf(ofp, "                               Needs to be a power of 2 
for\n");
-        fprintf(ofp, "                               kernel and ust 
tracers\n");
+    fprintf(ofp, "                               kernel and ust tracers.\n");
+    fprintf(ofp, "                               Hex values allowed using 0x 
prefix\n");
        fprintf(ofp, "      --num-subbuf NUM     Number of subbufers\n");
        fprintf(ofp, "                               (default: %u)\n",
                DEFAULT_CHANNEL_SUBBUF_NUM);
        fprintf(ofp, "                               Needs to be a power of 2 
for\n");
-        fprintf(ofp, "                               kernel and ust 
tracers\n");
+    fprintf(ofp, "                               kernel and ust tracers.\n");
+    fprintf(ofp, "                               Hex values allowed using 0x 
prefix\n");
        fprintf(ofp, "      --switch-timer USEC  Switch timer interval in usec 
(default: %u)\n",
                DEFAULT_CHANNEL_SWITCH_TIMER);
        fprintf(ofp, "      --read-timer USEC    Read timer interval in usec 
(default: %u)\n",
@@ -245,6 +247,8 @@ int cmd_enable_channels(int argc, const char **argv)
        int opt, ret = CMD_SUCCESS;
        static poptContext pc;
        char *session_name = NULL;
+       uint64_t lvalue;
+       char *argstr, *endptr;
 
        init_channel_config();
 
@@ -265,13 +269,111 @@ int cmd_enable_channels(int argc, const char **argv)
                        DBG("Channel set to overwrite");
                        break;
                case OPT_SUBBUF_SIZE:
-                       /* TODO Replace atol with strtol and check for errors */
-                       chan.attr.subbuf_size = atol(poptGetOptArg(pc));
-                       DBG("Channel subbuf size set to %" PRIu64, 
chan.attr.subbuf_size);
+                       argstr = poptGetOptArg(pc);
+                       errno = 0;
+                       lvalue = (uint64_t) strtoll(argstr, &endptr, 0); /* 
Allow hex or octal values */
+                       if (errno != 0)
+                       {
+                               ERR("Invalid channel subbuf size: %s", 
strerror(errno));
+                               usage(stderr);
+                               ret = CMD_ERROR;
+                               goto end;
+                       }
+                       else if (lvalue == 0 && argstr == endptr)
+                       {
+                               ERR("Invalid channel subbuf size: not a 
number");
+                               usage(stderr);
+                               ret = CMD_ERROR;
+                               goto end;
+                       }
+                       /* Human friendly imput */
+                       if (endptr != NULL && *endptr != '\0')
+                       {
+                               switch (*endptr) {
+                                       case 'k':
+                                       case 'K':
+                                               lvalue *= 1024;
+                                               break;
+                                       case 'm':
+                                       case 'M':
+                                               lvalue *= 1024*1024;
+                                               break;
+                                       case 'g':
+                                       case 'G':
+                                               lvalue *= 1024*1024*1024;
+                                               break;
+                                       case 't':
+                                       case 'T':
+                                               lvalue *= 1024*1024*1024*1024;
+                                               break;
+                               }
+                       }
+                       /*
+                        * Not power of 2
+                        * 
http://graphics.stanford.edu/~seander/bithacks.html#DetermineIfPowerOf2
+                        */
+                       if (!(lvalue && !(lvalue & (lvalue-1))))
+                       {
+                               /* Round up to power of 2 */
+                               lvalue--;
+                               lvalue |= lvalue >> 1; lvalue |= lvalue >> 2; 
lvalue |= lvalue >> 4;
+                               lvalue |= lvalue >> 8; lvalue |= lvalue >> 16; 
lvalue |= lvalue >> 32;
+                               lvalue++;
+                               /* and tell the user about it */
+                               MSG("Setting channel subbuf size to %"PRIu64" 
bytes", lvalue);
+                       }
+                       chan.attr.subbuf_size = lvalue;
+                       DBG("Setting channel subbuf size to %" PRIu64, 
chan.attr.subbuf_size);
                        break;
                case OPT_NUM_SUBBUF:
-                       /* TODO Replace atoi with strtol and check for errors */
-                       chan.attr.num_subbuf = atoi(poptGetOptArg(pc));
+                       argstr = poptGetOptArg(pc);
+                       errno = 0;
+                       lvalue = (uint64_t) strtoll(argstr, &endptr, 0); /* 
Allow hex or octal values  */
+                       if (errno != 0)
+                       {
+                               ERR("Invalid channel subbuf number: %s", 
strerror(errno));
+                               usage(stderr);
+                               ret = CMD_ERROR;
+                               goto end;
+                       }
+                       else if (lvalue == 0 && argstr == endptr)
+                       {
+                               ERR("Invalid channel subbuf number: not a 
number");
+                               usage(stderr);
+                               ret = CMD_ERROR;
+                               goto end;
+                       }
+                       if (endptr != NULL && *endptr != '\0')
+                       {
+                               switch (*endptr) {
+                                       case 'k':
+                                       case 'K':
+                                               lvalue *= 1024;
+                                               break;
+                                       case 'm':
+                                       case 'M':
+                                               lvalue *= 1024*1024;
+                                               break;
+                                       case 'g':
+                                       case 'G':
+                                               lvalue *= 1024*1024*1024;
+                                               break;
+                                       case 't':
+                                       case 'T':
+                                               lvalue *= 1024*1024*1024*1024;
+                                               break;
+                               }
+                       }
+                       if (!(lvalue && !(lvalue & (lvalue-1))))
+                       {
+                               lvalue--;
+                               lvalue |= lvalue >> 1; lvalue |= lvalue >> 2; 
lvalue |= lvalue >> 4;
+                               lvalue |= lvalue >> 8; lvalue |= lvalue >> 16; 
lvalue |= lvalue >> 32;
+                               lvalue++;
+                               /* and tell the user about it */
+                               MSG("Setting channel subbuf number to %" 
PRIu64, lvalue);
+                       }
+                       chan.attr.num_subbuf = lvalue;
                        DBG("Channel subbuf num set to %" PRIu64, 
chan.attr.num_subbuf);
                        break;
                case OPT_SWITCH_TIMER:

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to