<html><body><div style="font-family: arial, helvetica, sans-serif; font-size: 12pt; color: #000000"><div><span id="zwchr" data-marker="__DIVIDER__">----- On Nov 24, 2017, at 3:23 AM, Anders Wallin <wallinux@gmail.com> wrote:<br></span></div><div data-marker="__QUOTED_TEXT__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr">Hi,<br><div>architectures that has memory alignment restrictions may/will fail with the </div><div>optimization done in 51b8f2fa2b972e62117caa946dd3e3565b6ca4a3.</div><div>Please revert the patch or make it X86 specific.</div></div></blockquote><div><br></div><div>Hi Anders,<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div>This was added in the development cycle of lttng-ust 2.9. We could perhaps<br></div><div>add a test on the pointer alignment for architectures that care about it, and<br data-mce-bogus="1"></div><div>fallback to memcpy in those cases.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div>The revert approach would have been justified if this commit had been backported</div><div>as a "fix" to a stable branch, which is not the case here. We should work on<br></div><div>finding an acceptable solution that takes care of dealing with unaligned pointers<br></div><div>on architectures that care about the difference.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div>Thanks,<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div>Mathieu<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><br><div>Regards</div><br><div>Anders Wallin</div><div>------------------------------------------------------------------------------------------------------------</div><div><div>commit 51b8f2fa2b972e62117caa946dd3e3565b6ca4a3</div><div>Author: Mathieu Desnoyers <<a href="mailto:mathieu.desnoyers@efficios.com" target="_blank">mathieu.desnoyers@efficios.com</a>></div><div>Date: Sun Sep 25 12:31:11 2016 -0400</div><br><div> Performance: implement lttng_inline_memcpy</div><div> Because all length parameters received for serializing data coming from</div><div> applications go through a callback, they are never constant, and it</div><div> hurts performance to perform a call to memcpy each time.</div><div> Signed-off-by: Mathieu Desnoyers <<a href="mailto:mathieu.desnoyers@efficios.com" target="_blank">mathieu.desnoyers@efficios.com</a>></div><br><div>diff --git a/libringbuffer/backend_internal.h b/libringbuffer/backend_internal.h</div><div>index 90088b89..e597cf4d 100644</div><div>--- a/libringbuffer/backend_internal.h</div><div>+++ b/libringbuffer/backend_internal.h</div><div>@@ -592,6 +592,28 @@ int update_read_sb_index(const struct lttng_ust_lib_ring_buffer_config *config,</div><div> #define inline_memcpy(dest, src, n) memcpy(dest, src, n)</div><div> #endif</div><div>+static inline __attribute__((always_inline))</div><div>+void lttng_inline_memcpy(void *dest, const void *src,</div><div>+ unsigned long len)</div><div>+{</div><div>+ switch (len) {</div><div>+ case 1:</div><div>+ *(uint8_t *) dest = *(const uint8_t *) src;</div><div>+ break;</div><div>+ case 2:</div><div>+ *(uint16_t *) dest = *(const uint16_t *) src;</div><div>+ break;</div><div>+ case 4:</div><div>+ *(uint32_t *) dest = *(const uint32_t *) src;</div><div>+ break;</div><div>+ case 8:</div><div>+ *(uint64_t *) dest = *(const uint64_t *) src;</div><div>+ break;</div><div>+ default:</div><div>+ inline_memcpy(dest, src, len);</div><div>+ }</div><div>+}</div><div>+</div><div> /*</div><div> * Use the architecture-specific memcpy implementation for constant-sized</div><div> * inputs, but rely on an inline memcpy for length statically unknown.</div><div>@@ -603,7 +625,7 @@ do { \</div><div> if (__builtin_constant_p(len)) \</div><div> memcpy(dest, src, __len); \</div><div> else \</div><div>- inline_memcpy(dest, src, __len); \</div><div>+ lttng_inline_memcpy(dest, src, __len); \</div><div> } while (0)</div><div> /*</div></div><br><div>----------------------------------------------------------------------------------------------------</div><div>Here is one example of a crash, where 0xf3b7eeb2 is not 8 byte aligned;<br></div><br><div> (gdb) bt full</div><div>#0 lttng_inline_memcpy (len=8, src=0xf3b7eeb2, dest=<optimized out>) at /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend_internal.h:610</div><div>No locals.</div><div>#1 lib_ring_buffer_write (len=8, src=0xf3b7eeb2, ctx=0xf57c47d0, config=0xf737c560 <client_config>) at /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend.h:100</div><div> __len = 8</div><div> handle = 0xf3b2e0c0</div><div> backend_pages = <optimized out></div><div> chanb = 0xf3b2e2e0</div><div> offset = <optimized out></div><div>#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</div><div>No locals.</div><div>#3 0xf7337ef8 in ustctl_write_one_packet_to_channel (channel=<optimized out>, metadata_str=0xf3b7eeb2 "", len=<optimized out>) at /usr/src/debug/lttng-ust/2.9.1/git/liblttng-ust-ctl/ustctl.c:1183</div><div> 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' <repeats 11 times>, backend_pages = 0xf690c000}</div><div> chan = 0xf3b2e4d8</div><div> str = 0xf3b7eeb2 ""</div><div> reserve_len = 8</div><div> ret = <optimized out></div><div> __func__ = '\000' <repeats 34 times></div><div> __PRETTY_FUNCTION__ = '\000' <repeats 34 times></div><div>---Type <return> to continue, or q <return> to quit---</div><div>#4 0x000344cc in commit_one_metadata_packet (stream=stream@entry=0xf3b2e560) at ust-consumer.c:2206</div><div> write_len = <optimized out></div><div> ret = <optimized out></div><div> __PRETTY_FUNCTION__ = "commit_one_metadata_packet"</div><div>#5 0x00036538 in lttng_ustconsumer_read_subbuffer (stream=stream@entry=0xf3b2e560, ctx=ctx@entry=0x25e6e8) at ust-consumer.c:2452</div><div> len = 4096</div><div> subbuf_size = 4093</div><div> padding = <optimized out></div><div> err = -11</div><div> write_index = 1</div><div> ret = <optimized out></div><div> ustream = <optimized out></div><div> 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}</div><div> __PRETTY_FUNCTION__ = "lttng_ustconsumer_read_subbuffer"</div><div> __func__ = "lttng_ustconsumer_read_subbuffer"</div><div>.....</div></div><br>_______________________________________________<br>lttng-dev mailing list<br>lttng-dev@lists.lttng.org<br>https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev<br></blockquote></div><div><br></div><div data-marker="__SIG_POST__">-- <br></div><div>Mathieu Desnoyers<br>EfficiOS Inc.<br>http://www.efficios.com</div></div></body></html>