Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Beckius, Mikael via lttng-dev]

Hello Matthieu!

I have looked at this in place of Anders and as far as I can tell this is not an arm64 issue but an arm issue. And even on arm __ARM_FEATURE_UNALIGNED is 1 so it seems the problem only occurs if size equals 8.

In addition I did some performance testing of lttng_inline_memcpy by extracting it and adding it to a simple test program. It appears that the general performance increases on arm, arm64, arm on arm64 hardware and x86-64. But it also appears that on arm if you end up in memcpy the old code where you call memcpy directly is actually slightly faster.

Skipping the memcpy fallback on arm for unaligned copies of sizes 2 and 4 further improves the performance and setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 yields the best performance on arm64.

Micke
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

On 2023-01-30 01:50, Beckius, Mikael via lttng-dev wrote:
> Hello Matthieu!
> 
> I have looked at this in place of Anders and as far as I can tell this is not an arm64 issue but an arm issue. And even on arm __ARM_FEATURE_UNALIGNED is 1 so it seems the problem only occurs if size equals 8.

So for ARM, perhaps we should do the following in include/lttng/ust-arch.h:

#if defined(LTTNG_UST_ARCH_ARM) && defined(__ARM_FEATURE_UNALIGNED)
#define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
#endif

And refer to https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#ARM-Options

Based on that documentation, it is possible to build with -mno-unaligned-access,
and for all pre-ARMv6, all ARMv6-M and for ARMv8-M Baseline architectures,
unaligned accesses are not enabled.

I would only push this kind of change into the master branch though, due to
its impact and the fact that this is only a performance improvement.

> 
> In addition I did some performance testing of lttng_inline_memcpy by extracting it and adding it to a simple test program. It appears that the general performance increases on arm, arm64, arm on arm64 hardware and x86-64. But it also appears that on arm if you end up in memcpy the old code where you call memcpy directly is actually slightly faster.

Nothing unexpected here. Just make sure that your test program does not call lttng_inline_memcpy
with constant size values which end up optimizing away branches. In the context where lttng_inline_memcpy
is used, most of the time its arguments are not constants.

> 
> Skipping the memcpy fallback on arm for unaligned copies of sizes 2 and 4 further improves the performance

This would be naturally done on your board if we conditionally
set LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 for __ARM_FEATURE_UNALIGNED
right ?

and setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 yields the best performance on arm64.

This could go into lttng-ust master branch as well, e.g.:

#if defined(LTTNG_UST_ARCH_AARCH64)
#define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
#endif

Thanks!

Mathieu

> 
> Micke
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

On 2023-01-31 11:08, Mathieu Desnoyers wrote:
> On 2023-01-30 01:50, Beckius, Mikael via lttng-dev wrote:
>> Hello Matthieu!
>>
>> I have looked at this in place of Anders and as far as I can tell this 
>> is not an arm64 issue but an arm issue. And even on arm 
>> __ARM_FEATURE_UNALIGNED is 1 so it seems the problem only occurs if 
>> size equals 8.
> 
> So for ARM, perhaps we should do the following in include/lttng/ust-arch.h:
> 
> #if defined(LTTNG_UST_ARCH_ARM) && defined(__ARM_FEATURE_UNALIGNED)
> #define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
> #endif
> 
> And refer to 
> https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#ARM-Options
> 
> Based on that documentation, it is possible to build with 
> -mno-unaligned-access,
> and for all pre-ARMv6, all ARMv6-M and for ARMv8-M Baseline architectures,
> unaligned accesses are not enabled.
> 
> I would only push this kind of change into the master branch though, due to
> its impact and the fact that this is only a performance improvement.

But setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 for arm32
when __ARM_FEATURE_UNALIGNED is defined would still cause issues for
8-byte lttng_inline_memcpy with my proposed patch right ?

AFAIU 32-bit arm with __ARM_FEATURE_UNALIGNED has unaligned accesses for
2 and 4 bytes accesses, but somehow traps for unaligned 8-bytes
accesses ?

Thanks,

Mathieu

> 
>>
>> In addition I did some performance testing of lttng_inline_memcpy by 
>> extracting it and adding it to a simple test program. It appears that 
>> the general performance increases on arm, arm64, arm on arm64 hardware 
>> and x86-64. But it also appears that on arm if you end up in memcpy 
>> the old code where you call memcpy directly is actually slightly faster.
> 
> Nothing unexpected here. Just make sure that your test program does not 
> call lttng_inline_memcpy
> with constant size values which end up optimizing away branches. In the 
> context where lttng_inline_memcpy
> is used, most of the time its arguments are not constants.
> 
>>
>> Skipping the memcpy fallback on arm for unaligned copies of sizes 2 
>> and 4 further improves the performance
> 
> This would be naturally done on your board if we conditionally
> set LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 for 
> __ARM_FEATURE_UNALIGNED
> right ?
> 
> and setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 yields the 
> best performance on arm64.
> 
> This could go into lttng-ust master branch as well, e.g.:
> 
> #if defined(LTTNG_UST_ARCH_AARCH64)
> #define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
> #endif
> 
> Thanks!
> 
> Mathieu
> 
>>
>> Micke
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

On 2023-01-31 11:18, Mathieu Desnoyers wrote:
> On 2023-01-31 11:08, Mathieu Desnoyers wrote:
>> On 2023-01-30 01:50, Beckius, Mikael via lttng-dev wrote:
>>> Hello Matthieu!
>>>
>>> I have looked at this in place of Anders and as far as I can tell 
>>> this is not an arm64 issue but an arm issue. And even on arm 
>>> __ARM_FEATURE_UNALIGNED is 1 so it seems the problem only occurs if 
>>> size equals 8.
>>
>> So for ARM, perhaps we should do the following in 
>> include/lttng/ust-arch.h:
>>
>> #if defined(LTTNG_UST_ARCH_ARM) && defined(__ARM_FEATURE_UNALIGNED)
>> #define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
>> #endif
>>
>> And refer to 
>> https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#ARM-Options
>>
>> Based on that documentation, it is possible to build with 
>> -mno-unaligned-access,
>> and for all pre-ARMv6, all ARMv6-M and for ARMv8-M Baseline 
>> architectures,
>> unaligned accesses are not enabled.
>>
>> I would only push this kind of change into the master branch though, 
>> due to
>> its impact and the fact that this is only a performance improvement.
> 
> But setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 for arm32
> when __ARM_FEATURE_UNALIGNED is defined would still cause issues for
> 8-byte lttng_inline_memcpy with my proposed patch right ?
> 
> AFAIU 32-bit arm with __ARM_FEATURE_UNALIGNED has unaligned accesses for
> 2 and 4 bytes accesses, but somehow traps for unaligned 8-bytes
> accesses ?

Re-reading your analysis, I may have mistakenly concluded that using the
lttng ust ring buffer in "packed" mode would be faster than aligned mode 
on arm32 and aarch64, but that's not really what you have benchmarked there.

So forget what I said about setting 
LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS to 1 for arm32 and aarch64.

There is a distinction between having efficient unaligned access and
supporting unaligned accesses at all.

For aarch64, it appears to support unaligned accesses, but it may be
slower than aligned accesses AFAIU.

For arm32, it supports unaligned accesses for 2 and 4 bytes when 
__ARM_FEATURE_UNALIGNED is set, but not for 8 bytes (it traps). Then 
it's not clear whether a 2 or 4 bytes access is slower when unaligned 
compared to aligned.

At the end of the day, it's a question of compactness of the generated 
trace data (added throughput overhead) vs cpu time required to perform 
an unaligned access vs aligned.

Thoughts ?

Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
>>
>>>
>>> In addition I did some performance testing of lttng_inline_memcpy by 
>>> extracting it and adding it to a simple test program. It appears that 
>>> the general performance increases on arm, arm64, arm on arm64 
>>> hardware and x86-64. But it also appears that on arm if you end up in 
>>> memcpy the old code where you call memcpy directly is actually 
>>> slightly faster.
>>
>> Nothing unexpected here. Just make sure that your test program does 
>> not call lttng_inline_memcpy
>> with constant size values which end up optimizing away branches. In 
>> the context where lttng_inline_memcpy
>> is used, most of the time its arguments are not constants.
>>
>>>
>>> Skipping the memcpy fallback on arm for unaligned copies of sizes 2 
>>> and 4 further improves the performance
>>
>> This would be naturally done on your board if we conditionally
>> set LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 for 
>> __ARM_FEATURE_UNALIGNED
>> right ?
>>
>> and setting LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1 yields the 
>> best performance on arm64.
>>
>> This could go into lttng-ust master branch as well, e.g.:
>>
>> #if defined(LTTNG_UST_ARCH_AARCH64)
>> #define LTTNG_UST_ARCH_HAS_EFFICIENT_UNALIGNED_ACCESS 1
>> #endif
>>
>> Thanks!
>>
>> Mathieu
>>
>>>
>>> Micke
>>> _______________________________________________
>>> lttng-dev mailing list
>>> lttng-dev@lists.lttng.org
>>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

Hi  Mikael,

I just tried another approach to fix this issue, see:

https://review.lttng.org/c/lttng-ust/+/9413 Fix: use unaligned pointer accesses for lttng_inline_memcpy

It is less intrusive than other approaches, and does not change the generated code on the
most relevant architectures.

Feedback is welcome,

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Beckius, Mikael via lttng-dev]

Hello Mathieu!

Sorry for the late reply. I was away for a few days.

I will have a look at your updated approach and get back to you on your other replies if still relevant, but in short:
- With __ARM_FEATURE_UNALIGNED defined 32-bit arm appears to support 2 and 4 bytes unaligned access
- Regarding 8 bytes I found this wording:
   "__ARM_FEATURE_UNALIGNED is defined if the target supports unaligned access in hardware, at least to the extent of being able
   to load or store an integer word at any alignment with a single instruction. (There may be restrictions on load-multiple and
   floating-point accesses.)" on https://developer.arm.com/documentation/101028/0012/5--Feature-test-macros
   and I think all the crash reports were about 8 bytes unaligned access on arm 32-bit
- Performance seems to improve for both aligned and unaligned access compared to using memcpy but your are right that a test needs to be carefully constructed
- 64-bit arm appears to support 2, 4 and 8 bytes unaligned access

Micke
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Beckius, Mikael via lttng-dev]

Hello Mathieu!

I added your latest implementation to my test and it seems to perform well on both arm and arm64. Since the test was written in C++ I had to make a small change to the cast in order for the test to compile.

Micke

> -----Ursprungligt meddelande-----
> Från: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Skickat: den 2 februari 2023 17:26
> Till: Beckius, Mikael <mikael.beckius@windriver.com>; lttng-
> dev@lists.lttng.org
> Ämne: Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch
> specific optimization
> 
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
> 
> Hi  Mikael,
> 
> I just tried another approach to fix this issue, see:
> 
> https://review.lttng.org/c/lttng-ust/+/9413 Fix: use unaligned pointer
> accesses for lttng_inline_memcpy
> 
> It is less intrusive than other approaches, and does not change the generated
> code on the
> most relevant architectures.
> 
> Feedback is welcome,
> 
> Thanks,
> 
> Mathieu
> 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

Hi Micke,

I did tweaks to make the code C++ compatible even though it's currently 
only built in C. It makes it more future-proof.

I've merged the resulting patch into lttng-ust 
master/stable-2.13/stable-2.12. Thanks for testing !

Mathieu

On 2023-02-06 11:15, Beckius, Mikael wrote:
> Hello Mathieu!
> 
> I added your latest implementation to my test and it seems to perform well on both arm and arm64. Since the test was written in C++ I had to make a small change to the cast in order for the test to compile.
> 
> Micke
> 
>> -----Ursprungligt meddelande-----
>> Från: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Skickat: den 2 februari 2023 17:26
>> Till: Beckius, Mikael <mikael.beckius@windriver.com>; lttng-
>> dev@lists.lttng.org
>> Ämne: Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch
>> specific optimization
>>
>> CAUTION: This email comes from a non Wind River email account!
>> Do not click links or open attachments unless you recognize the sender and
>> know the content is safe.
>>
>> Hi  Mikael,
>>
>> I just tried another approach to fix this issue, see:
>>
>> https://review.lttng.org/c/lttng-ust/+/9413 Fix: use unaligned pointer
>> accesses for lttng_inline_memcpy
>>
>> It is less intrusive than other approaches, and does not change the generated
>> code on the
>> most relevant architectures.
>>
>> Feedback is welcome,
>>
>> Thanks,
>>
>> Mathieu
>>
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> https://www.efficios.com
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

On 2023-01-26 14:32, Anders Wallin wrote:
> Hi Matthieu,
> 
> I've retired and no longer have access to any arch64  target to test it on.
> 

Thanks for your reply Anders,

I've talked to Henrik and Pär today and they are already testing it out.

Enjoy your retirement :)

Best regards,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Anders Wallin via lttng-dev]

[-- Attachment #1.1: Type: text/plain, Size: 11446 bytes --]

Hi Matthieu,

I've retired and no longer have access to any arch64  target to test it on.

Regards
Anders


Den ons 25 jan. 2023 13:25Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
skrev:

> Hi Anders,
>
> Sorry for the long delay on this one, can you have a look at the following
> fix ?
>
> https://review.lttng.org/c/lttng-ust/+/9319 Fix: aarch64: do not perform
> unaligned stores
>
> If it passes your testing, I'll merge this into lttng-ust.
>
> Thanks,
>
> Mathieu
>
> On 2017-12-28 09:13, Anders Wallin wrote:
> > Hi Mathieu,
> >
> > I finally got some time to dig into this issue. The crash only happens
> > when metadata is written AND the size of the metadata will end up in a
> > write that is 8,4,2 or 1 bytes long AND
> > that the source or destination is not aligned correctly according to HW
> > limitation. I have not found any simple way to keep the performance
> > enhancement code that is run most of the time.
> > Maybe the metadata writes should have it's own write function instead.
> >
> > Here is an example of a crash (code is from lttng-ust 2.9.1 and
> > lttng-tools 2.9.6) where the size is 8 bytes and the src address is
> > unaligned at 0xf3b7eeb2;
> >
> > #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
> > No locals.
> > #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
> >          __len = 8
> >          handle = 0xf3b2e0c0
> >          backend_pages = <optimized out>
> >          chanb = 0xf3b2e2e0
> >          offset = <optimized out>
> >
> > #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=<optimized
> > out>, metadata_str=0xf3b7eeb2 "", len=<optimized out>) 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' <repeats 11
> > times>, backend_pages = 0xf690c000}
> >          chan = 0xf3b2e4d8
> >          str = 0xf3b7eeb2 ""
> >          reserve_len = 8
> >          ret = <optimized out>
> >          __func__ = '\000' <repeats 34 times>
> >          __PRETTY_FUNCTION__ = '\000' <repeats 34 times>
> > ---Type <return> to continue, or q <return> to quit---
> >
> > #4  0x000344cc in commit_one_metadata_packet
> > (stream=stream@entry=0xf3b2e560) at ust-consumer.c:2206
> >          write_len = <optimized out>
> >          ret = <optimized out>
> >          __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 = <optimized out>
> >          err = -11
> >          write_index = 1
> >          ret = <optimized out>
> >          ustream = <optimized out>
> >          index = {offset = 0, packet_size = 575697416355872,
> > content_size = 17564043391468256584, timestamp_begin =
> > 17564043425827782792, timestamp_end = 34359738496,
> > Regards
> > Anders
> >
> > fre 24 nov. 2017 kl 20:18 skrev Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com <mailto:mathieu.desnoyers@efficios.com
> >>:
> >
> >     ----- On Nov 24, 2017, at 3:23 AM, Anders Wallin <wallinux@gmail.com
> >     <mailto:wallinux@gmail.com>> 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 <mathieu.desnoyers@efficios.com
> >         <mailto:mathieu.desnoyers@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
> >         <mathieu.desnoyers@efficios.com
> >         <mailto:mathieu.desnoyers@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=<optimized
> >         out>) 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 <client_config>) at
> >         /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend.h:100
> >                  __len = 8
> >                  handle = 0xf3b2e0c0
> >                  backend_pages = <optimized out>
> >                  chanb = 0xf3b2e2e0
> >                  offset = <optimized out>
> >         #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=<optimized out>, metadata_str=0xf3b7eeb2 "",
> >         len=<optimized out>) 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' <repeats 11 times>, backend_pages =
> >         0xf690c000}
> >                  chan = 0xf3b2e4d8
> >                  str = 0xf3b7eeb2 ""
> >                  reserve_len = 8
> >                  ret = <optimized out>
> >                  __func__ = '\000' <repeats 34 times>
> >                  __PRETTY_FUNCTION__ = '\000' <repeats 34 times>
> >         ---Type <return> to continue, or q <return> to quit---
> >         #4  0x000344cc in commit_one_metadata_packet
> >         (stream=stream@entry=0xf3b2e560) at ust-consumer.c:2206
> >                  write_len = <optimized out>
> >                  ret = <optimized out>
> >                  __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 = <optimized out>
> >                  err = -11
> >                  write_index = 1
> >                  ret = <optimized out>
> >                  ustream = <optimized out>
> >                  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 mailing list
> >         lttng-dev@lists.lttng.org <mailto:lttng-dev@lists.lttng.org>
> >         https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >         <https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev>
> >
> >
> >     --
> >     Mathieu Desnoyers
> >     EfficiOS Inc.
> >     http://www.efficios.com <http://www.efficios.com>
> >
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
>
>

[-- Attachment #1.2: Type: text/html, Size: 16579 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] lttng-consumerd crash on aarch64 due to x86 arch specific optimization

[Mathieu Desnoyers via lttng-dev]

Hi Anders,

Sorry for the long delay on this one, can you have a look at the following fix ?

https://review.lttng.org/c/lttng-ust/+/9319 Fix: aarch64: do not perform unaligned stores

If it passes your testing, I'll merge this into lttng-ust.

Thanks,

Mathieu

On 2017-12-28 09:13, Anders Wallin wrote:
> Hi Mathieu,
> 
> I finally got some time to dig into this issue. The crash only happens 
> when metadata is written AND the size of the metadata will end up in a 
> write that is 8,4,2 or 1 bytes long AND
> that the source or destination is not aligned correctly according to HW 
> limitation. I have not found any simple way to keep the performance 
> enhancement code that is run most of the time.
> Maybe the metadata writes should have it's own write function instead.
> 
> Here is an example of a crash (code is from lttng-ust 2.9.1 and 
> lttng-tools 2.9.6) where the size is 8 bytes and the src address is 
> unaligned at 0xf3b7eeb2;
> 
> #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
> No locals.
> #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
>          __len = 8
>          handle = 0xf3b2e0c0
>          backend_pages = <optimized out>
>          chanb = 0xf3b2e2e0
>          offset = <optimized out>
> 
> #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=<optimized 
> out>, metadata_str=0xf3b7eeb2 "", len=<optimized out>) 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' <repeats 11 
> times>, backend_pages = 0xf690c000}
>          chan = 0xf3b2e4d8
>          str = 0xf3b7eeb2 ""
>          reserve_len = 8
>          ret = <optimized out>
>          __func__ = '\000' <repeats 34 times>
>          __PRETTY_FUNCTION__ = '\000' <repeats 34 times>
> ---Type <return> to continue, or q <return> to quit---
> 
> #4  0x000344cc in commit_one_metadata_packet 
> (stream=stream@entry=0xf3b2e560) at ust-consumer.c:2206
>          write_len = <optimized out>
>          ret = <optimized out>
>          __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 = <optimized out>
>          err = -11
>          write_index = 1
>          ret = <optimized out>
>          ustream = <optimized out>
>          index = {offset = 0, packet_size = 575697416355872, 
> content_size = 17564043391468256584, timestamp_begin = 
> 17564043425827782792, timestamp_end = 34359738496,
> Regards
> Anders
> 
> fre 24 nov. 2017 kl 20:18 skrev Mathieu Desnoyers 
> <mathieu.desnoyers@efficios.com <mailto:mathieu.desnoyers@efficios.com>>:
> 
>     ----- On Nov 24, 2017, at 3:23 AM, Anders Wallin <wallinux@gmail.com
>     <mailto:wallinux@gmail.com>> 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 <mathieu.desnoyers@efficios.com
>         <mailto:mathieu.desnoyers@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
>         <mathieu.desnoyers@efficios.com
>         <mailto:mathieu.desnoyers@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=<optimized
>         out>) 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 <client_config>) at
>         /usr/src/debug/lttng-ust/2.9.1/git/libringbuffer/backend.h:100
>                  __len = 8
>                  handle = 0xf3b2e0c0
>                  backend_pages = <optimized out>
>                  chanb = 0xf3b2e2e0
>                  offset = <optimized out>
>         #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=<optimized out>, metadata_str=0xf3b7eeb2 "",
>         len=<optimized out>) 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' <repeats 11 times>, backend_pages =
>         0xf690c000}
>                  chan = 0xf3b2e4d8
>                  str = 0xf3b7eeb2 ""
>                  reserve_len = 8
>                  ret = <optimized out>
>                  __func__ = '\000' <repeats 34 times>
>                  __PRETTY_FUNCTION__ = '\000' <repeats 34 times>
>         ---Type <return> to continue, or q <return> to quit---
>         #4  0x000344cc in commit_one_metadata_packet
>         (stream=stream@entry=0xf3b2e560) at ust-consumer.c:2206
>                  write_len = <optimized out>
>                  ret = <optimized out>
>                  __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 = <optimized out>
>                  err = -11
>                  write_index = 1
>                  ret = <optimized out>
>                  ustream = <optimized out>
>                  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 mailing list
>         lttng-dev@lists.lttng.org <mailto:lttng-dev@lists.lttng.org>
>         https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>         <https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev>
> 
> 
>     -- 
>     Mathieu Desnoyers
>     EfficiOS Inc.
>     http://www.efficios.com <http://www.efficios.com>
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev