Skip to content

common: give lockdep's group name to OpenSSL's mutexes#28987

Merged
liewegas merged 5 commits intoceph:masterfrom
rzarzynski:wip-bug-40698-2
Jul 17, 2019
Merged

common: give lockdep's group name to OpenSSL's mutexes#28987
liewegas merged 5 commits intoceph:masterfrom
rzarzynski:wip-bug-40698-2

Conversation

@rzarzynski
Copy link
Contributor

Fixes: http://tracker.ceph.com/issues/40698
Signed-off-by: Radoslaw Zarzynski rzarzyns@redhat.com

@LenzGr LenzGr requested review from liewegas and tchaikov July 11, 2019 13:18
@sebastian-philipp
Copy link
Contributor

I've enjoyed reading https://gist.github.com/rzarzynski/b2e73a4fefd86c1cbbafc76f8e3ee55f . Thanks for the write-up!

@liewegas
Copy link
Member

I don't understand why openssl is making use of the ceph debug mutex...

IMO passing an empty name to the lock ctor is the bug. That shouldn't have gotten past review in BlueFS (my bad there), although to be fair the additional args there are supposed to disable lockdep.

@sebastian-philipp
Copy link
Contributor

I don't understand why openssl is making use of the ceph debug mutex...

Because of https://github.com/ceph/ceph/blob/master/src/common/ceph_crypto.cc#L97

@liewegas
Copy link
Member

I don't understand why openssl is making use of the ceph debug mutex...

Because of https://github.com/ceph/ceph/blob/master/src/common/ceph_crypto.cc#L97

Ah. We should be able to construct those with names like "ssl-%d" then, right?

@rzarzynski
Copy link
Contributor Author

Ah. We should be able to construct those with names like "ssl-%d" then, right?

Working on that. There is some pain with non-moveability of mutexes after combining with 1) our abstractions (ceph::make_shared_mutex) for hiding construction differences between std::shared_mutex and ceph::shared_mutex_debug; 2) the run-time nature of OpenSSL's CRYPTO_num_locks().

Possibly will need to introduce some new boilerplate.

@alfonsomthd
Copy link
Contributor

Tested locally on CentOS 7:
The issue is not reproducible anymore.

@rzarzynski rzarzynski force-pushed the wip-bug-40698-2 branch 4 times, most recently from fd6cde9 to 34d0edf Compare July 13, 2019 19:57
@rzarzynski rzarzynski changed the title common: enforce name uniquity across debug mutexes. common: give lockdep's group name to OpenSSL's mutexes Jul 13, 2019
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Fixes: http://tracker.ceph.com/issues/40698
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
This reverts commit d92117d
at the rason for it has been fixed.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
@rzarzynski
Copy link
Contributor Author

jenkins retest this please (TableComponent › should prevent propagation of mouseenter event)

@liewegas
Copy link
Member

assuming no backport for this? @rzarzynski

@liewegas liewegas merged commit 8ba6775 into ceph:master Jul 17, 2019
liewegas added a commit that referenced this pull request Jul 17, 2019
* refs/pull/28987/head:
	Revert "mgr/dashboard: Add nolockdep option to e2e-script"
	common: refactor handling of lockdep's group name in debug locks.
	common: shared_mutex_debug doesn't use empty group name for lockdep.
	common, crypto: give names to OpenSSL's mutexes.
	common: tiny_vector – CPU-friendly container for mutexes & co.

Reviewed-by: Kefu Chai <kchai@redhat.com>
@rzarzynski
Copy link
Contributor Author

assuming no backport for this? @rzarzynski

Yup, no backport needed as #27834 is hasn't been backported as well.

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.

5 participants