Fix: consumerd: order of metadata cache vs stream lock
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Mon, 9 Jan 2017 16:23:16 +0000 (11:23 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Mon, 9 Jan 2017 16:59:55 +0000 (11:59 -0500)
The locking order comment in consumer.h is incorrect. First, its
description of locking order is not in sync with the comment found in
consumer-metadata-cache.h. The comment in struct consumer_metadata_cache
only states that the metadata cache lock nests inside the consumer_data
lock, and does not mention the stream lock, which implies that the
metadata cache lock does NOT nest inside the stream lock. But let's
investigate further to confirm:

* lttng_consumer_read_subbuffer() acquires the stream lock, and then
  calls lttng_ustconsumer_read_subbuffer() with stream lock held,
  and then invokes commin_one_metadata_packet(), which acquires the
  metadata cache lock.

* lttng_ustconsumer_sync_metadata() acquires the metadata stream lock,
  and calls commit_one_metadata_packet(), which takes the metadata cache
  lock.

Therefore, update the comment in consumer.h to state that the metadata
cache lock nests INSIDE the stream lock, and update
consumer_del_metadata_stream() accordingly.

This should take care of fixing the locking order reversal found by
Coverity.

CID 1368314 (#1 of 1): Thread deadlock (ORDER_REVERSAL)
CID 1368319:  Program hangs  (ORDER_REVERSAL)

Fixes: 5feafd4130 "Fix: protect the channel's metadata stream using the metadata cache lock"
Fixes: 1ea6cc572b "Fix: lock nesting order reversed"
Fixes: fb549e7ac2 "Fix: reverse channel and metadata cache lock nesting order"
Reported-by: Coverity Scan
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

No differences found
This page took 0.025031 seconds and 4 git commands to generate.