Replace explicit rcu_read_lock/unlock with lttng::urcu::read_lock_guard
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Sat, 28 Jan 2023 09:54:09 +0000 (04:54 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 2 Feb 2023 22:06:41 +0000 (17:06 -0500)
commit56047f5a23df5c2c583a102b8015bbec5a7da9f1
tree9ee97f452040c052dc206b2ee1a15e6481fead3f
parent66cefebdc240cbae0bc79594305f509b0779fa98
Replace explicit rcu_read_lock/unlock with lttng::urcu::read_lock_guard

The explicit use of rcu_read_lock/unlock() has been a frequent source of
bugs in the code base as the locks can often end-up imbalanced when
error paths are taken (through goto's).

Since lttng::urcu::read_lock_guard was implemented in a previous commit,
replace all usages of the rcu_read_lock/unlock() API with an RAII
lock_guard wrapper.

Essentially, lttng::urcu::read_lock_guard acquires the RCU reader lock
on construction, and releases it when it goes out of scope. This
automates what is accomplished in the various error paths by jumping to
error handling labels.

For more info, see: https://en.cppreference.com/w/cpp/thread/lock_guard

No user-visible change of behaviour is intended by this patch. The scope
of some critical sections has been reduced when possible and when it
didn't matter from a correctness standpoint. The RCU reader lock would
often be held longer than necessary to simplify the error paths.

Explicit scopes are used to limit the lifetime of the reader lock guards
when used in long functions or when it is only intended to protect the
iteration over cds_lfht instances. In those cases, the following pattern
is used:

```cpp
void foo()
{
  [...]
  {
    lttng::urcu::read_lock_guard read_guard;

    cds_lfht_for_each(...) {
      [...]
  }
  [...]
}
```

This explicitly bounds the critical section and also serves as a hint as
to why the read guard is being used. It is fairly verbose, but I think
it's a good compromise until we add an idiomatic urcu wrapper that
automates this stuff too.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie3ef8ddf0f1ab813971522176115688939696370
45 files changed:
src/bin/lttng-relayd/connection.cpp
src/bin/lttng-relayd/ctf-trace.cpp
src/bin/lttng-relayd/index.cpp
src/bin/lttng-relayd/live.cpp
src/bin/lttng-relayd/main.cpp
src/bin/lttng-relayd/session.cpp
src/bin/lttng-relayd/sessiond-trace-chunks.cpp
src/bin/lttng-relayd/stream.cpp
src/bin/lttng-relayd/viewer-session.cpp
src/bin/lttng-relayd/viewer-stream.cpp
src/bin/lttng-sessiond/action-executor.cpp
src/bin/lttng-sessiond/agent-thread.cpp
src/bin/lttng-sessiond/agent.cpp
src/bin/lttng-sessiond/buffer-registry.cpp
src/bin/lttng-sessiond/channel.cpp
src/bin/lttng-sessiond/cmd.cpp
src/bin/lttng-sessiond/consumer.cpp
src/bin/lttng-sessiond/context.cpp
src/bin/lttng-sessiond/dispatch.cpp
src/bin/lttng-sessiond/event-notifier-error-accounting.cpp
src/bin/lttng-sessiond/event.cpp
src/bin/lttng-sessiond/kernel-consumer.cpp
src/bin/lttng-sessiond/kernel.cpp
src/bin/lttng-sessiond/lttng-syscall.cpp
src/bin/lttng-sessiond/manage-kernel.cpp
src/bin/lttng-sessiond/notification-thread-events.cpp
src/bin/lttng-sessiond/rotation-thread.cpp
src/bin/lttng-sessiond/save.cpp
src/bin/lttng-sessiond/session.cpp
src/bin/lttng-sessiond/snapshot.cpp
src/bin/lttng-sessiond/trace-ust.cpp
src/bin/lttng-sessiond/tracker.cpp
src/bin/lttng-sessiond/ust-app.cpp
src/bin/lttng-sessiond/ust-consumer.cpp
src/common/channel.cpp
src/common/consumer/consumer-stream.cpp
src/common/consumer/consumer-timer.cpp
src/common/consumer/consumer.cpp
src/common/fd-tracker/fd-tracker.cpp
src/common/fd-tracker/inode.cpp
src/common/hashtable/hashtable.cpp
src/common/kernel-consumer/kernel-consumer.cpp
src/common/macros.hpp
src/common/trace-chunk.cpp
src/common/ust-consumer/ust-consumer.cpp
This page took 0.032031 seconds and 4 git commands to generate.