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

Add locking to atomic operations in rw/rcu tests #24001

Closed
wants to merge 2 commits into from

Conversation

nhorman
Copy link
Contributor

@nhorman nhorman commented Mar 29, 2024

I neglected to add locks to the calls to CRYPTO_atomic_add in these test, which on newer compilers is fine, as atomic operations are defined. However on older compilers the __ATOMIC_ACQ_REL definition is missing causing these function to be implemented using an rwlock, which when NULL causes the locks to fail.

Fix this my creating the lock and using them appropriately

@nhorman nhorman self-assigned this Mar 29, 2024
@nhorman nhorman added branch: master Merge to master branch branch: 3.3 Merge to openssl-3.3 labels Mar 29, 2024
@mattcaswell
Copy link
Member

Can we add "Fixes #24000" in the commit line?

crypto/sleep.c Outdated Show resolved Hide resolved
@mattcaswell mattcaswell added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Mar 29, 2024
I neglected to add locks to the calls to CRYPTO_atomic_add in these
test, which on newer compilers is fine, as atomic operations are
defined.  However on older compilers the __ATOMIC_ACQ_REL definition is
missing causing these function to be implemented using an rwlock, which
when NULL causes the locks to fail.

Fix this my creating the lock and using them appropriately
Add locking to atomic operations in rw/rcu tests

I neglected to add locks to the calls to CRYPTO_atomic_add in these
test, which on newer compilers is fine, as atomic operations are
defined.  However on older compilers the __ATOMIC_ACQ_REL definition is
missing causing these function to be implemented using an rwlock, which
when NULL causes the locks to fail.

Fix this my creating the lock and using them appropriately

Fixes openssl#24000
@paulidale paulidale removed the approval: otc review pending This pull request needs review by an OTC member label Mar 30, 2024
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 1, 2024
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Apr 2, 2024
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@nhorman
Copy link
Contributor Author

nhorman commented Apr 2, 2024

merged

@nhorman nhorman closed this Apr 2, 2024
openssl-machine pushed a commit that referenced this pull request Apr 2, 2024
I neglected to add locks to the calls to CRYPTO_atomic_add in these
test, which on newer compilers is fine, as atomic operations are
defined.  However on older compilers the __ATOMIC_ACQ_REL definition is
missing causing these function to be implemented using an rwlock, which
when NULL causes the locks to fail.

Fix this my creating the lock and using them appropriately

Fixes #24000

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #24001)
@t8m t8m added the triaged: bug The issue/pr is/fixes a bug label Apr 3, 2024
@t8m
Copy link
Member

t8m commented Apr 11, 2024

Cherry picked to 3.3 branch as it was approved for it.

openssl-machine pushed a commit that referenced this pull request Apr 11, 2024
I neglected to add locks to the calls to CRYPTO_atomic_add in these
test, which on newer compilers is fine, as atomic operations are
defined.  However on older compilers the __ATOMIC_ACQ_REL definition is
missing causing these function to be implemented using an rwlock, which
when NULL causes the locks to fail.

Fix this my creating the lock and using them appropriately

Fixes #24000

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from #24001)

(cherry picked from commit 5f8b812)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.3 Merge to openssl-3.3 triaged: bug The issue/pr is/fixes a bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3.3.0-alpha1: CPU loop in threadstest (torture_rw_low) on old Linux RHEL6 and SLES11
5 participants