refCount alignment issue

[Svelar]

Describe the bug
IN cachelib/allocator/Refcount.h, even

// why not use std::atomic? Because std::atomic does not work well with
// padding and requires alignment.
attribute((aligned(alignof(Value)))) Value refCount_{0};

made, on ARM when using atomic ops like __atomic_or_fetch, __atomic_and_fetch, alignment are still required. The reason is ARM can't do atomic ops across cacheline compared with what X86 act. Bus error would be appeared when testing such as allocator-test-AllocatorTypeTest because of unaligned allocSize. Any ideas?

To Reproduce
Steps to reproduce the behavior:

1. ARM machine
2. Build test
3. Run allocator-test-AllocatorTypeTest
4. See bus error

Expected behavior
Pass tests

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[jaesoo-fb]

Hi @Svelar

Thanks for the report. What is the exact processor architecture? Just in case, is it 32 bit architecture?

As explained here, the refcount is third members and should start aligned on 4B since the access hook and mm hook are both aligned on 8B as long as the alloc sizes are aligned on 4B. In fact, generateAllocSizes(...) should give all alloc sizes aligned on 8B.

So, this might be just a test issue where we are passing some arbitrary alloc sizes (e.g., 50B) for testing. I think we can enforce the 4B alignment of alloc sizes and amend the tests accordingly for now.

For disclaimer, we are not testing on ARM for now, so the build and the operation are not guaranteed. We might need to consider the ARM support in the future for various reasons, but we always welcome any contribution in the meantime.

[Svelar]

Hi @jaesoo-fb , thanks for your reply.

I tested on kunpeng 920 (aarch64) server.

Yes, you are right. Nothing is affected but tests. Actually, I modified a little bit codes to enforce allocSize aligned on 8B and worked fine. I will raise a PR once it passed all tests.

I tested on ARM with centOS and openEuler, and there are not many codes should be added. Pin me if I could offer any help!

[jaesoo-fb]

Thanks for the update. We really appreciate if you can submit it as PR.

Just in case, is this related to another issue #276 as well?

[Svelar]

No really, that is caused by mistake of 'compiler optimization'. I still need more time to verify that.

But you could close this issue anytime, thanks!