- On Nov 24, 2017, at 3:23 AM, Anders Wallin wrote:
> Hi,
> architectures that has memory alignment restrictions may/will fail with the
> optimization done in 51b8f2fa2b972e62117caa946dd3e3565b6ca4a3.
> Please revert the patch or make it X86 specific.
Hi Anders,
This was added in the development cycle of lttng-ust 2.9. We could perhaps
add a test on the pointer alignment for architectures that care about it, and
fallback to memcpy in those cases.
The revert approach would have been justified if this commit had been
backported
as a "fix" to a stable branch, which is not the case here. We should work on
finding an acceptable solution that takes care of dealing with unaligned
pointers
on architectures that care about the difference.
Thanks,
Mathieu
> Regards
> Anders Wallin
>
> commit 51b8f2fa2b972e62117caa946dd3e3565b6ca4a3
> Author: Mathieu Desnoyers < [ mailto:mathieu.desnoy...@efficios.com |
> mathieu.desnoy...@efficios.com ] >
> Date: Sun Sep 25 12:31:11 2016 -0400
> Performance: implement lttng_inline_memcpy
> Because all length parameters received for serializing data coming from
> applications go through a callback, they are never constant, and it
> hurts performance to perform a call to memcpy each time.
> Signed-off-by: Mathieu Desnoyers < [ mailto:mathieu.desnoy...@efficios.com |
> mathieu.desnoy...@efficios.com ] >
> diff --git a/libringbuffer/backend_internal.h
> b/libringbuffer/backend_internal.h
> index 90088b89..e597cf4d 100644
> --- a/libringbuffer/backend_internal.h
> +++ b/libringbuffer/backend_internal.h
> @@ -592,6 +592,28 @@ int update_read_sb_index(const struct
> lttng_ust_lib_ring_buffer_config *config,
> #define inline_memcpy(dest, src, n) memcpy(dest, src, n)
> #endif
> +static inline __attribute__((always_inline))
> +void lttng_inline_memcpy(void *dest, const void *src,
> + unsigned long len)
> +{
> + switch (len) {
> + case 1:
> + *(uint8_t *) dest = *(const uint8_t *) src;
> + break;
> + case 2:
> + *(uint16_t *) dest = *(const uint16_t *) src;
> + break;
> + case 4:
> + *(uint32_t *) dest = *(const uint32_t *) src;
> + break;
> + case 8:
> + *(uint64_t *) dest = *(const uint64_t *) src;
> + break;
> + default:
> + inline_memcpy(dest, src, len);
> + }
> +}
> +
> /*
> * Use the architecture-specific memcpy implementation for constant-sized
> * inputs, but rely on an inline memcpy for length statically unknown.
> @@ -603,7 +625,7 @@ do { \
> if (__builtin_constant_p(len)) \
> memcpy(dest, src, __len); \
> else \
> - inline_memcpy(dest, src, __len); \
> + lttng_inline_memcpy(dest, src, __len); \
> } while (0)
> /*
>
> Here is one example of a crash, where 0xf3b7eeb2 is not 8 byte aligned;
> (gdb) bt full
> #0 lttng_inline_memcpy (len=8, src=0xf3b7eeb2, dest=) at
> /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend_internal.h:610
> No locals.
> #1 lib_ring_buffer_write (len=8, src=0xf3b7eeb2, ctx=0xf57c47d0,
> config=0xf737c560 ) at
> /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend.h:100
> __len = 8
> handle = 0xf3b2e0c0
> backend_pages =
> chanb = 0xf3b2e2e0
> offset =
> #2 lttng_event_write (ctx=0xf57c47d0, src=0xf3b7eeb2, len=8) at
> /usr/src/debug/lttng-ust/2.9.1/git/liblttng-ust/lttng-ring-buffer-metadata-client.h:267
> No locals.
> #3 0xf7337ef8 in ustctl_write_one_packet_to_channel (channel=,
> metadata_str=0xf3b7eeb2 "", len=) at
> /usr/src/debug/lttng-ust/2.9.1/git/liblttng-ust-ctl/ustctl.c:1183
> ctx = {chan = 0xf3b2e290, priv = 0x0, handle = 0xf3b2e0c0, data_size = 8,
> largest_align = 1, cpu = -1, buf = 0xf6909000, slot_size = 8, buf_offset =
> 163877, pre_offset = 163877, tsc = 0, rflags = 0, ctx_len = 80, ip = 0x0,
> priv2
> = 0x0, padding2 = '\000' , backend_pages = 0xf690c000}
> chan = 0xf3b2e4d8
> str = 0xf3b7eeb2 ""
> reserve_len = 8
> ret =
> __func__ = '\000'
> __PRETTY_FUNCTION__ = '\000'
> ---Type to continue, or q to quit---
> #4 0x000344cc in commit_one_metadata_packet (stream=stream@entry=0xf3b2e560)
> at
> ust-consumer.c:2206
> write_len =
> ret =
> __PRETTY_FUNCTION__ = "commit_one_metadata_packet"
> #5 0x00036538 in lttng_ustconsumer_read_subbuffer
> (stream=stream@entry=0xf3b2e560, ctx=ctx@entry=0x25e6e8) at
> ust-consumer.c:2452
> len = 4096
> subbuf_size = 4093
> padding =
> err = -11
> write_index = 1
> ret =
> ustream =
> index = {offset = 0, packet_size = 575697416355872, content_size =
> 17564043391468256584, timestamp_begin = 17564043425827782792, timestamp_end =
> 34359738496, events_discarded = 313327092840, stream_id = 4089446416,
> stream_instance_id = 17689097291346376608, packet_seq_num = 8589934593}
> __PRETTY_FUNCTION__ = "lttng_ustconsumer_read_subbuffer"
> __func__ = "lttng_ustconsumer_read_subbuffer"
> .
> ___
> lttng-dev ma
