Skip to content
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.

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

[v4.1.x] fix atomic operations opal_atomic_compare_exchange_strong_ in arm64 #12005

Closed
wants to merge 1 commit into from

Conversation

yuncliu
Copy link

@yuncliu yuncliu commented Oct 18, 2023

fix opal_atomic_compare_exchange_strong_ bug in arm64

@github-actions github-actions bot added this to the v4.1.7 milestone Oct 18, 2023
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

4f38ed6: fix atomic operation and spinlock bug

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

1 similar comment
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

4f38ed6: fix atomic operation and spinlock bug

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

@jsquyres
Copy link
Member

Is this a v4.1.x port of #11999? If so, can you finish the #11999 PR and get that merged first, and then cherry-pick it over here to the v4.1.x branch?

Thanks!

@jsquyres jsquyres changed the title fix atomic operation and spinlock bug v4.1.x: fix atomic operation and spinlock bug Oct 18, 2023
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

4f38ed6: fix atomic operation and spinlock bug

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

Signed-off-by: liuyuncheng <[email protected]>
@yuncliu yuncliu force-pushed the v4.1.x_atomic_fix3 branch from 4f38ed6 to 2bb9b5b Compare October 20, 2023 12:37
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

2bb9b5b: [arm64] fix atomic strong

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

@yuncliu yuncliu closed this Oct 20, 2023
@yuncliu yuncliu reopened this Oct 20, 2023
@yuncliu yuncliu changed the title v4.1.x: fix atomic operation and spinlock bug [arm64] fix atomic operations opal_atomic_compare_exchange_strong_ Oct 20, 2023
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

2bb9b5b: [arm64] fix atomic strong

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

1 similar comment
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

2bb9b5b: [arm64] fix atomic strong

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

@yuncliu yuncliu changed the title [arm64] fix atomic operations opal_atomic_compare_exchange_strong_ [v4.1.x] fix atomic operations opal_atomic_compare_exchange_strong_ in arm64 Oct 20, 2023
@github-actions
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

2bb9b5b: [arm64] fix atomic strong

  • check_cherry_pick: does not include a cherry pick message (did you need to bot:notacherrypick?)

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

@yuncliu
Copy link
Author

yuncliu commented Oct 20, 2023

Is this a v4.1.x port of #11999? If so, can you finish the #11999 PR and get that merged first, and then cherry-pick it over here to the v4.1.x branch?

Thanks!

The main branch only need to modify one file, but 4.1.x need to modify two atomic.h with same implementation, cherry-pick will lost the change in ./opal/include/opal/sys/arm64/atomic.h

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

It looks like we do have a problem here in v4.1.x somewhere, but this is apparently not the desired solution (per discussion on #11999). I'm marking this as "request changes" to ensure we don't merge it, but don't want to close it yet.

#12011 is a new issue opened to track the problem (on main, v5.0.x, and v4.1.x).

@lrbison
Copy link
Contributor

lrbison commented Feb 21, 2024

@yuncliu thank you again for reporting this issue. I think it may be fixed in #12338. Can we have any further discussion in the issue #12011 rather than this PR?

I will close this PR for now, as I don't think it is the correct change. However if you could confirm the additional write memory barrier in smcuda fixed your issue #12011 it would be greatly appreciated!

Thank you

@lrbison lrbison closed this Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants