From 45e9e6996ae8da8d5dfffbe9aebd8e0438a84124 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 13 Nov 2011 17:16:20 -0500 Subject: [PATCH] Fix fork handling, where child shares shm with parent Consider that consumer does most of the finalize/flush, so we don't need to perform them in teardown. Signed-off-by: Mathieu Desnoyers --- liblttng-ust/lttng-ust-abi.c | 20 ++++++++- libringbuffer/ring_buffer_backend.c | 36 +--------------- libringbuffer/ring_buffer_frontend.c | 62 ++++------------------------ 3 files changed, 28 insertions(+), 90 deletions(-) diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c index 6a60532d..435c86a6 100644 --- a/liblttng-ust/lttng-ust-abi.c +++ b/liblttng-ust/lttng-ust-abi.c @@ -32,6 +32,8 @@ #include "lttng/core.h" #include "ltt-tracer.h" +static int lttng_ust_abi_close_in_progress; + static int lttng_abi_tracepoint_list(void); @@ -836,7 +838,21 @@ int lttng_rb_release(int objd) buf = priv->buf; channel = priv->ltt_chan; free(priv); - channel->ops->buffer_read_close(buf, channel->handle); + /* + * If we are at ABI exit, we don't want to close the + * buffer opened for read: it is being shared between + * the parent and child (right after fork), and we don't + * want the child to close it for the parent. For a real + * exit, we don't care about marking it as closed, as + * the consumer daemon (if there is one) will do fine + * even if we don't mark it as "closed" for reading on + * our side. + * We only mark it as closed if it is being explicitely + * released by the session daemon with an explicit + * release command. + */ + if (!lttng_ust_abi_close_in_progress) + channel->ops->buffer_read_close(buf, channel->handle); return lttng_ust_objd_unref(channel->objd); } @@ -900,5 +916,7 @@ static const struct lttng_ust_objd_ops lttng_event_ops = { void lttng_ust_abi_exit(void) { + lttng_ust_abi_close_in_progress = 1; objd_table_destroy(); + lttng_ust_abi_close_in_progress = 0; } diff --git a/libringbuffer/ring_buffer_backend.c b/libringbuffer/ring_buffer_backend.c index c32ba982..23802e82 100644 --- a/libringbuffer/ring_buffer_backend.c +++ b/libringbuffer/ring_buffer_backend.c @@ -129,14 +129,6 @@ int lib_ring_buffer_backend_create(struct lttng_ust_lib_ring_buffer_backend *buf handle, shmobj); } -void lib_ring_buffer_backend_free(struct lttng_ust_lib_ring_buffer_backend *bufb) -{ - /* bufb->buf_wsb will be freed by shm teardown */ - /* bufb->array[i] will be freed by shm teardown */ - /* bufb->array will be freed by shm teardown */ - bufb->allocated = 0; -} - void lib_ring_buffer_backend_reset(struct lttng_ust_lib_ring_buffer_backend *bufb, struct lttng_ust_shm_handle *handle) { @@ -311,15 +303,6 @@ int channel_backend_init(struct channel_backend *chanb, return 0; free_bufs: - if (config->alloc == RING_BUFFER_ALLOC_PER_CPU) { - for_each_possible_cpu(i) { - struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chanb->buf[i].shmp); - - if (!buf->backend.allocated) - continue; - lib_ring_buffer_free(buf, handle); - } - } /* We only free the buffer data upon shm teardown */ end: return -ENOMEM; @@ -334,24 +317,7 @@ end: void channel_backend_free(struct channel_backend *chanb, struct lttng_ust_shm_handle *handle) { - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; - unsigned int i; - - if (config->alloc == RING_BUFFER_ALLOC_PER_CPU) { - for_each_possible_cpu(i) { - struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chanb->buf[i].shmp); - - if (!buf->backend.allocated) - continue; - lib_ring_buffer_free(buf, handle); - } - } else { - struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chanb->buf[0].shmp); - - CHAN_WARN_ON(chanb, !buf->backend.allocated); - lib_ring_buffer_free(buf, handle); - } - /* We only free the buffer data upon shm teardown */ + /* SHM teardown takes care of everything */ } /** diff --git a/libringbuffer/ring_buffer_frontend.c b/libringbuffer/ring_buffer_frontend.c index f8c106c6..53d269d2 100644 --- a/libringbuffer/ring_buffer_frontend.c +++ b/libringbuffer/ring_buffer_frontend.c @@ -83,26 +83,15 @@ struct switch_offsets { __thread unsigned int lib_ring_buffer_nesting; +/* + * TODO: this is unused. Errors are saved within the ring buffer. + * Eventually, allow consumerd to print these errors. + */ static void lib_ring_buffer_print_errors(struct channel *chan, struct lttng_ust_lib_ring_buffer *buf, int cpu, struct lttng_ust_shm_handle *handle); -/* - * Must be called under cpu hotplug protection. - */ -void lib_ring_buffer_free(struct lttng_ust_lib_ring_buffer *buf, - struct lttng_ust_shm_handle *handle) -{ - struct channel *chan = shmp(handle, buf->backend.chan); - - lib_ring_buffer_print_errors(chan, buf, buf->backend.cpu, handle); - /* buf->commit_hot will be freed by shm teardown */ - /* buf->commit_cold will be freed by shm teardown */ - - lib_ring_buffer_backend_free(&buf->backend); -} - /** * lib_ring_buffer_reset - Reset ring buffer to initial values. * @buf: Ring buffer. @@ -231,7 +220,6 @@ free_init: free_commit: /* commit_hot will be freed by shm teardown */ free_chanbuf: - lib_ring_buffer_backend_free(&buf->backend); return ret; } @@ -596,9 +584,6 @@ void channel_release(struct channel *chan, struct lttng_ust_shm_handle *handle, void channel_destroy(struct channel *chan, struct lttng_ust_shm_handle *handle, int shadow) { - const struct lttng_ust_lib_ring_buffer_config *config = &chan->backend.config; - int cpu; - if (shadow) { channel_release(chan, handle, shadow); return; @@ -606,42 +591,11 @@ void channel_destroy(struct channel *chan, struct lttng_ust_shm_handle *handle, channel_unregister_notifiers(chan, handle); - if (config->alloc == RING_BUFFER_ALLOC_PER_CPU) { - for_each_channel_cpu(cpu, chan) { - struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[cpu].shmp); - - if (config->cb.buffer_finalize) - config->cb.buffer_finalize(buf, - channel_get_private(chan), - cpu, handle); - if (buf->backend.allocated) - lib_ring_buffer_switch_slow(buf, SWITCH_FLUSH, - handle); - /* - * Perform flush before writing to finalized. - */ - cmm_smp_wmb(); - CMM_ACCESS_ONCE(buf->finalized) = 1; - //wake_up_interruptible(&buf->read_wait); - } - } else { - struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[0].shmp); + /* + * Note: the consumer takes care of finalizing and switching the + * buffers. + */ - if (config->cb.buffer_finalize) - config->cb.buffer_finalize(buf, channel_get_private(chan), -1, handle); - if (buf->backend.allocated) - lib_ring_buffer_switch_slow(buf, SWITCH_FLUSH, - handle); - /* - * Perform flush before writing to finalized. - */ - cmm_smp_wmb(); - CMM_ACCESS_ONCE(buf->finalized) = 1; - //wake_up_interruptible(&buf->read_wait); - } - CMM_ACCESS_ONCE(chan->finalized) = 1; - //wake_up_interruptible(&chan->hp_wait); - //wake_up_interruptible(&chan->read_wait); /* * sessiond/consumer are keeping a reference on the shm file * descriptor directly. No need to refcount. -- 2.34.1