On Sun, Jul 7, 2019 at 11:32 PM kefu chai <tchaikov(a)gmail.com> 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