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 261a0ad3f8a1bf0ae4af6e76c9a2a246115589b4..9a6c8ee920fa0dee16fa4f086534cbc99f4348d0 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 0512a3f3ddfda66a72cf468f8f8c4730e2e4d4a1..98992f9f7943047f967233edffb4083df39585b8 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 037467da655dcfe06cb477e7ad27b99696995286..3e8f80935e6449a196d88adc3a0ed2fcb77599d2 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 {