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 64af55fb24938db7491a39483dd2071d0c7ede3d..cffebcd8b0fc60a646101d02553bc87a269a6272 100644 (file)
--- 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 701cff8ca8ff8663f82c0148bfb4ac15448bf81c..1fecff8efc279a76de3c8344f23236d485f7e5b8 100644 (file)
--- 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 e1de1af8974da959230ab8f1920d92f290f2a23b..b09bd4fceaf352d7f1508c1cd90e8c40203a7cae 100644 (file)
--- 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 {