lttng-ust:stable-2.11

Last commit made on 2019-05-17
Get this branch:
git clone -b stable-2.11 https://git.launchpad.net/lttng-ust

Branch merges

Branch information

Name:
stable-2.11
Repository:
lp:lttng-ust

Recent commits

2d6d0b4... by Mathieu Desnoyers on 2019-05-17

Cleanup: bitfields: streamline use of underscores

Do not prefix macro arguments with underscores. Use one leading
underscore as prefix for local variables defined within macros.

Signed-off-by: Mathieu Desnoyers <email address hidden>

3268eea... by Mathieu Desnoyers on 2019-05-14

Silence compiler "always false comparison" warning

Compiling the bitfield test with gcc -Wextra generates those warnings:

 ../../include/babeltrace/bitfield-internal.h:38:45: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
 #define _bt_is_signed_type(type) ((type) -1 < (type) 0)

This is the intent of the macro. Disable compiler warnings around use of
that macro.

Signed-off-by: Mathieu Desnoyers <<email address hidden>

fa81c5d... by Mathieu Desnoyers on 2019-05-14

Fix: bitfield: shift undefined/implementation defined behaviors

bitfield.h uses the left shift operator with a left operand which
may be negative. The C99 standard states that shifting a negative
value is undefined.

When building with -Wshift-negative-value, we get this gcc warning:

In file included from /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
                 from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In function ‘bt_ctfser_write_unsigned_int’:
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24: error: left shift of negative value [-Werror=shift-negative-value]
   mask = ~((~(type) 0) << (__start % ts)); \
                        ^
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2: note: in expansion of macro ‘_bt_bitfield_write_le’
  _bt_bitfield_write_le(ptr, type, _start, _length, _v)
  ^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3: note: in expansion of macro ‘bt_bitfield_write_le’
   bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
   ^~~~~~~~~~~~~~~~~~~~

This boils down to the fact that the expression ~((uint8_t)0) has type
"signed int", which is used as an operand of the left shift. This is due
to the integer promotion rules of C99 (6.3.3.1):

    If an int can represent all values of the original type, the value is
    converted to an int; otherwise, it is converted to an unsigned int.
    These are called the integer promotions. All other types are unchanged
    by the integer promotions.

We also need to cast the result explicitly into the left hand
side type to deal with:

warning: large integer implicitly truncated to unsigned type [-Woverflow]

The C99 standard states that a right shift has implementation-defined
behavior when shifting a signed negative value. Add a preprocessor check
that the compiler provides the expected behavior, else provide an
alternative implementation which guarantees the intended behavior.

A preprocessor check is also added to ensure that the compiler
representation for signed values is two's complement, which is expected
by this header.

Document that this header strictly respects the C99 standard, with
the exception of its use of __typeof__.

Signed-off-by: Mathieu Desnoyers <email address hidden>

28098fa... by Stefan Wallentowitz on 2019-05-10

Fix: Update coding style link

The documentation at kernel.org changed and the coding style has
moved.

Signed-off-by: Stefan Wallentowitz <email address hidden>

9cce407... by Mathieu Desnoyers on 2019-05-10

Fix: alignment of ring buffer shm space reservation

commit a9ff648cc "Implement file-backed ring buffer" changes the order
of backend fields with respect to the frontend per-subbuffer
commit_counters_hot and commit_counters_cold arrays, but does not change
that order when calculating the space needed in the initial pass.

This discrepancy can be an issue for field alignment calculation.
Let's analyse the situation. If the incorrect position of alignment
calculation leads to a larger space reserved than the actual
allocations, no ill effect will be perceived by the user. However,
if space calculation is less than the allocations, it will cause the
ring buffer (and thus channel) creation to fail.

The fields that are incorrectly misplaced in size calculation (in
officially released versions) are:

* struct commit_counters_hot is aligned on CAA_CACHE_LINE_SIZE,
* struct commit_counters_cold is aligned on CAA_CACHE_LINE_SIZE,

Those are placed after (should be before) the backend fields:

* struct lttng_ust_lib_ring_buffer_backend_pages_shmp aligned on the
  natural alignment of ssize_t,
* alignment on page size,
* struct lttng_ust_lib_ring_buffer_backend_pages, aligned on the natural
  alignment of ssize_t,
* struct lttng_ust_lib_ring_buffer_backend_subbuffer, aligned on natural
  alignment of unsigned long,
* struct lttng_ust_lib_ring_buffer_backend_counts, aligned on natural
  alignment of uint64_t.

The largest alignment is the alignment on page size in the backend
fields. If we have a channel configured within specific ranges of
sub-buffer count, we should reach commit counters array dimensions
which cause the page size alignment to be lower than it should be in
the space calculation, and therefore leads to a problematic scenario
where space allocation will fail, thus leading to channel creation
failures.

Signed-off-by: Mathieu Desnoyers <email address hidden>

dba2bc1... by Gabriel-Andrew Pollo-Guilbert <email address hidden> on 2019-05-10

Fix: allocate ts_end in ringbuffer shared memory

Allocate the memory used by the ts_end field added by commit 6c737d05.
When allocating lots of subbuffer for a channel (512 or more),
zalloc_shm() will fail to allocate all the objects because the allocated
memory map didn't take account the newly added field.

With lttng-tools version: b14f53d4 (2.12.0-pre)

Steps to reproduce the bug:

 1. lttng-sessiond -vvv --verbose-consumer
 2. start a traced application
 3. lttng create "test-sesssion"
 4. lttng enable-channel --userspace --num-subbuf 512 \
                                --subbuf-size 8k --overwrite channel
 5. lttng enable-event -u -a -c channel
 6. lttng start

After these steps, the following error message show should be thrown:

 Error: ask_channel_creation consumer command failed
 Error: Error creating UST channel "channel" on the consumer daemon

Signed-off-by: Gabriel-Andrew Pollo-Guilbert <email address hidden>
Signed-off-by: Mathieu Desnoyers <email address hidden>

74e4d0d... by Mathieu Desnoyers on 2019-04-30

Fix: timestamp_end field should include all events within sub-buffer

Fix for timestamp_end not including all events within sub-buffer. This
happens if a thread is preempted/interrupted for a long time between
reserve and commit (e.g. in the middle of a packet), which causes the
timestamp used for timestamp_end field of the packet header to be lower
than the timestamp of the last events in the buffer (those following the
event that was preempted/interrupted between reserve and commit).

The fix involves sampling the timestamp when doing the last space
reservation in a sub-buffer (which necessarily happens before doing the
delivery after its last commit). Save this timestamp temporarily in a
per-sub-buffer control area (we have exclusive access to that area until
we increment the commit counter).

Then, that timestamp value will be read when delivering the sub-buffer,
whichever event or switch happens to be the last to increment the commit
counter to perform delivery. The timestamp value can be read without
worrying about concurrent access, because at that point sub-buffer
delivery has exclusive access to the sub-buffer.

This ensures the timestamp_end value is always larger or equal to the
timestamp of the last event, always below or equal the timestamp_begin
of the following packet, and always below or equal the timestamp of the
first event in the following packet.

This changes the layout of the ring buffer shared memory area, so we
need to bump the LTTNG_UST_ABI version from 7.2 to 8.0, thus requiring
locked-step upgrade between liblttng-ust in applications, session
daemon, and consumer daemon. This fix therefore cannot be backported
to existing stable releases.

Fixes: #1183
Signed-off-by: Mathieu Desnoyers <email address hidden>

c9b22e4... by Mathieu Desnoyers on 2019-04-10

ust-ctl API: clarify getter usage requirements

Signed-off-by: Mathieu Desnoyers <email address hidden>

ad73be8... by Mathieu Desnoyers on 2019-04-10

Fix: don't access packet header for stream_id and stream_instance_id getters

The stream ID and stream instance ID are invariant for a stream, so
there is no point reading them from the packet header currently owned by
the consumer (between get/put subbuf).

Actually, the consumer try to access the stream_id from the live timer
when sending a live beacon without getting the reader subbuffer first.
Doing so is racy against producers. In typical live scenarios
(non-overwrite channels), the producers will always write the same
stream id and stream instance id values at the same header offsets,
which will "work", except for the initial state of an empty buffer:
the value "0" will be returned (erroneously).

For the less frequently used scenario of a live session with "overwrite"
channels, this is handled by issuing a CHAN_WARN_ON, which disables
tracing for the channel, and prints warning to the consumerd console
when running consumerd with LTTNG_UST_DEBUG=1.

In the case where a ring buffer does not have any data ready, it makes
no sense to try to get a subbuffer for reading anyway, so the approach
was broken.

So return the stream id and stream instance id from the internal
data structures rather than reading it from the ring buffer.

Signed-off-by: Mathieu Desnoyers <email address hidden>

4dfde2b... by Michael Jeanson <email address hidden> on 2019-03-20

compat: work around broken _SC_NPROCESSORS_CONF on MUSL libc

On MUSL libc the _SC_NPROCESSORS_CONF sysconf will report the number of
CPUs allocated to the task based on the affinity mask instead of the
total number of CPUs configured on the system.

Signed-off-by: Michael Jeanson <email address hidden>
Signed-off-by: Mathieu Desnoyers <email address hidden>