From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Fri, 26 Sep 2014 00:33:21 +0000 (-0400)
Subject: Fix: handle concurrent flush vs get_next_subbuf on metadata cache
X-Git-Tag: v2.5.1~15
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=b773bad5f8ed0cbf0623b766d3c2bc4910b6e0cf;p=lttng-modules.git

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 <mathieu.desnoyers@efficios.com>
---

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 {