Skip to content

Update cpu relax functions #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 20, 2021
Merged

Update cpu relax functions #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3dc07d
Update UT_RELAX_CPU to match upstream changes in MySQL
AGSaidi Aug 16, 2021
d1322c4
Fix cpu_relax and add RELAX_IS_ISB
AGSaidi Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ext/linux/include/lk_atomics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,9 @@ static inline uint16_t xchg_release16(uint16_t *ptr, uint16_t val) {
static inline void cpu_relax (void) {
#if defined(__x86_64__)
asm volatile ("pause" : : : "memory" );
#elif defined (__arch64__)
#elif defined (__aarch64__) && defined(RELAX_IS_ISB)
Copy link
Contributor

@rpgolshan rpgolshan Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do you recommend a user enabling this over the default? Can you add a define in the Makefile?

Copy link
Contributor Author

@AGSaidi AGSaidi Sep 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the define, this behaves like the kernel does today. I believe the define improves the performance of the kernel locks but it's not what is committed today.

Copy link
Contributor

@rpgolshan rpgolshan Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so if the kernel changes in the future we just need to expose this define.

asm volatile ("isb" : : : "memory" );
#elif defined (__aarch64__)
asm volatile ("yield" : : : "memory" );
#endif
}
Expand Down
4 changes: 2 additions & 2 deletions ext/mysql/include/ut_atomics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ this program; if not, write to the Free Software Foundation, Inc.,

#if defined(__x86_64__)
#define UT_RELAX_CPU() asm volatile ("rep; nop")
#elif defined(__AARCH64__)
#elif defined(__aarch64__)
// Theoretically we could emit a yield here but MySQL doesn't do it
// and most ARM cores are likely to NOP it anyway
#define UT_RELAX_CPU() asm volatile ("":::"memory")
#define UT_RELAX_CPU() asm volatile ("isb":::"memory")
Copy link
Contributor

@rpgolshan rpgolshan Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine.

#else
#define UT_RELAX_CPU() asm volatile ("":::"memory")
#endif