From de23d59d5992b7354b57a533b4d582809ef8d43c Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 25 Sep 2014 20:33:21 -0400 Subject: [PATCH] Fix: handle concurrent flush vs get_next_subbuf on metadata cache The "flush" operation can be performed on the metadata file descriptor concurrently with get_next_subbuffer operations by different processes (e.g. lttng session daemon issuing flush at "stop" concurrently with consumer daemon issuing get_next_subbuf for metadata I/O). We need to protect the metadata cache from those concurrent use by introducing a mutex. This fixes a race where metadata from a kernel trace is corrupted due to this scenario. The corruption shows up like the same metadata content (a metadata packet) being written twice consecutively in the metadata stream, thus triggering a babeltrace "parse error" when trying to read the trace. Signed-off-by: Mathieu Desnoyers --- lttng-abi.c | 1 + lttng-events.c | 11 ++++++++--- lttng-events.h | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lttng-abi.c b/lttng-abi.c index 64af55fb..cffebcd8 100644 --- a/lttng-abi.c +++ b/lttng-abi.c @@ -841,6 +841,7 @@ int lttng_abi_open_metadata_stream(struct file *channel_file) metadata_stream->priv = buf; stream_priv = metadata_stream; metadata_stream->transport = channel->transport; + mutex_init(&metadata_stream->lock); /* * Since life-time of metadata cache differs from that of diff --git a/lttng-events.c b/lttng-events.c index 701cff8c..1fecff8e 100644 --- a/lttng-events.c +++ b/lttng-events.c @@ -597,16 +597,20 @@ int lttng_metadata_output_channel(struct lttng_metadata_stream *stream, /* * Ensure we support mutiple get_next / put sequences followed - * by put_next. + * by put_next. The metadata stream lock internally protects + * reading the metadata cache. It can indeed be read + * concurrently by "get_next_subbuf" and "flush" operations on + * the buffer invoked by different processes. */ + mutex_lock(&stream->lock); WARN_ON(stream->metadata_in < stream->metadata_out); if (stream->metadata_in != stream->metadata_out) - return 0; + goto end; len = stream->metadata_cache->metadata_written - stream->metadata_in; if (!len) - return 0; + goto end; reserve_len = min_t(size_t, stream->transport->ops.packet_avail_size(chan), len); @@ -628,6 +632,7 @@ int lttng_metadata_output_channel(struct lttng_metadata_stream *stream, ret = reserve_len; end: + mutex_unlock(&stream->lock); return ret; } diff --git a/lttng-events.h b/lttng-events.h index e1de1af8..b09bd4fc 100644 --- a/lttng-events.h +++ b/lttng-events.h @@ -311,6 +311,7 @@ struct lttng_metadata_stream { wait_queue_head_t read_wait; /* Reader buffer-level wait queue */ struct list_head list; /* Stream list */ struct lttng_transport *transport; + struct mutex lock; }; struct lttng_session { -- 2.34.1