-
Notifications
You must be signed in to change notification settings - Fork 6k
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
common: fix lockdep vs recursive mutexes #9940
Conversation
Do you mind creating a unit test for this? We don't actually have any lockdep unit tests, but just a few basic ones would be really nice. (e.g., demonstrate that a nonrecursive AA or ABBA will assert, and that a recursive AA is okay. gtest has the assert macros that assert that the test aborts.) |
Ok, I will gladly write some unit tests. |
LGTM, but needs a rebase now that we're cmake! |
@aclamk ping? |
2074376
to
a28a063
Compare
@tchaikov rebased. |
*/ | ||
|
||
#include <common/Mutex.h> | ||
#include "gtest/gtest.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to update the CMakeLists.txt
to include this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tchaikov: The unittest_mutex_debug is already a part of src/test/common/CMakeLists.txt, lines 191-194. Can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aclamk it's "test_mutex_debug.cc" not "test_mutex.cc".
could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes |
a28a063
to
1e8a878
Compare
ec0ed2a
to
b8ef1bf
Compare
retest this please. |
can you drop the ceph-disk patch from this PR please? |
…heck asserts. Signed-off-by: Adam Kupczyk <[email protected]>
Signed-off-by: Adam Kupczyk <[email protected]>
Signed-off-by: Adam Kupczyk <[email protected]>
b8ef1bf
to
b08d688
Compare
@liewegas Dropped ceph-disk patch. I have no idea how it even could get there.... |
The problem is that:
Mutex lock("RGWKeystoneTokenCache", true /* recursive */);
Mutex::Locker l(lock);
lock.Lock();
fails...
Signed-off-by: Adam Kupczyk [email protected]