On Sun, Jul 7, 2019 at 11:32 PM kefu chai <tchaikov@xxxxxxxxx> wrote: > > hi Patrick and list, > > i am trying replace Mutex with ceph::mutex in Ceph. the goal is to > deprecate and then remove Mutex and Cond from our project. as we have > already ceph::mutex and ceph::condition_variable. it's confusing and, > in the long run, i think, it will hurt us as a technical debt. > > most of the refactory work is quite straightforward, and we can always > replace Mutex::is_locked_by_me() with ceph_mutex_is_locked_by_me(). > this macro will be expanded to `true` in release build. as we think it > will be used only by `ceph_assert()` and `assert()`. > > but i realized that we are also using Mutex::is_locked_by_me() in > functional code which won't be optimised out. for instance, in > MDSRank::MDSRank() we check "mds_lock.is_locked_by_me()", if it's > true, we just `handle_write_error(r)` without acquiring the lock, > otherwise, we will call this function within `mds_lock`. the same > applies to `MDSDaemon::handle_conf_change()`. so i am wondering if > it's okay to change `mds_lock` to a recursive lock, so we don't need > to query this information from > the mutex. > > what do you think? handle_conf_change should be easy enough to fix by using a finisher context like we do for the Monitor. handle_write_error() is trickier because we can't easily pass the locker down the stack since it's a callback from the osdc/Journaler. I think we can find a way to avoid this issue via some reworking; a recursive mutex is not really desirable to resolve this one edge-case. I'll create a tracker for this. -- Patrick Donnelly, Ph.D. He / Him / His Senior Software Engineer Red Hat Sunnyvale, CA GPG: 19F28A586F808C2402351B93C3301A3E258DD79D _______________________________________________ Dev mailing list -- dev@xxxxxxx To unsubscribe send an email to dev-leave@xxxxxxx
