On Tue, 13 Mar 2018, Sage Weil wrote:
If we /do/ want to keep lockdep around, then the start of the work there
highlights the specific roles that each Mutex and Cond observes. There is in
no case any actual shift in runtime behavior that I saw in going through 219
source files, I'm very confident that we can succeed in this.
I think we definitely *do* want to keep lockdep, but make it a
compile-time option. Make vstart dev checkouts default to building with
the debug mutex, but production builds with std::mutex.
Ok, this makes sense. The /good/ news is that I'm very confident that
ceph::Mutex can be redesigned in concrete terms that will allow for this.
The type system can basically help! And the extra-good news is that with
C++17 this is much more straightforward than before.
// JFW: these are race conditions waiting to happen... should be done away
with!
bool is_locked() const { return nlock > 0; }
This one definitely has to die.
Agreed, there's no way to guarantee it. Thankfully, I only saw IIRC one or
two places where we "depend" on it.
bool is_locked_by_me() const { return is_locked() && this_thread::get_id()
== locked_by; }
This one is safe in that there is never a false negative, so you can
assert(is_locked_by_me()). You cannot assert(!is_locked_by_me()), though.
Ok, true!
It sounds like you and Adam are pretty set on a more correct check that
involves passing through a unique_lock to do the check (vs my suggestion
of a macro that is a no-op in the non-debug build case). I have no
objection to the former as long as it doesn't overly pollute the code... I
worry it has a runtime cost for production builds, though!
I think, again, that with constexpr-if we have an avenue for avoiding
runtime overhead here. Basically, no macros but you still wind up with a
no-op! At least, I'm pretty optimistic!
-Jesse
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
