From f0fde1c3984721c0660c7d5a1377db74b6c03a70 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 18 Mar 2021 09:34:55 -0400 Subject: [PATCH] struct lttng_channel: split protocol ABI from instrumentation ABI Duplicate struct lttng_channel to introduce a new struct lttng_ust_abi_channel_config meant to hold configuration data within the memory area belonging to the ring buffer private data. Now the application and consumer private pointer will be a separate pointer, for which the memory will be owned by the caller. This will allow us to modify the layout of struct lttng_channel without breaking the UST communication protocol. Signed-off-by: Mathieu Desnoyers Change-Id: I96684aa6658f59dc3332da6034bdf9e3f921261f --- liblttng-ust/lttng-events.c | 5 +--- liblttng-ust/lttng-ring-buffer-client.h | 24 ++++++++++++----- .../lttng-ring-buffer-metadata-client.h | 22 ++++++++++----- liblttng-ust/lttng-ust-abi.c | 18 +++++++++++-- liblttng-ust/ust-events-internal.h | 26 ++++++++++++++++++ libringbuffer/frontend.h | 9 +++---- libringbuffer/frontend_types.h | 17 ++++++++++-- libringbuffer/ring_buffer_frontend.c | 27 ++++++++++--------- 8 files changed, 109 insertions(+), 39 deletions(-) diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index 625d98af..d5fd9fee 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -239,11 +239,8 @@ void _lttng_channel_unmap(struct lttng_channel *lttng_chan) lttng_destroy_context(lttng_chan->ctx); chan = lttng_chan->chan; handle = lttng_chan->handle; - /* - * note: lttng_chan is private data contained within handle. It - * will be freed along with the handle. - */ channel_destroy(chan, handle, 0); + free(lttng_chan); } static diff --git a/liblttng-ust/lttng-ring-buffer-client.h b/liblttng-ust/lttng-ring-buffer-client.h index 188efb0f..f791cd1e 100644 --- a/liblttng-ust/lttng-ring-buffer-client.h +++ b/liblttng-ust/lttng-ring-buffer-client.h @@ -649,33 +649,43 @@ struct lttng_channel *_channel_create(const char *name, const int *stream_fds, int nr_stream_fds, int64_t blocking_timeout) { - struct lttng_channel chan_priv_init; + struct lttng_ust_abi_channel_config chan_priv_init; struct lttng_ust_shm_handle *handle; struct lttng_channel *lttng_chan; - void *priv; + + lttng_chan = zmalloc(sizeof(struct lttng_channel)); + if (!lttng_chan) + return NULL; + memcpy(lttng_chan->uuid, uuid, LTTNG_UST_UUID_LEN); + lttng_chan->id = chan_id; memset(&chan_priv_init, 0, sizeof(chan_priv_init)); memcpy(chan_priv_init.uuid, uuid, LTTNG_UST_UUID_LEN); chan_priv_init.id = chan_id; + handle = channel_create(&client_config, name, - &priv, __alignof__(struct lttng_channel), - sizeof(struct lttng_channel), + __alignof__(struct lttng_ust_abi_channel_config), + sizeof(struct lttng_ust_abi_channel_config), &chan_priv_init, - buf_addr, subbuf_size, num_subbuf, + lttng_chan, buf_addr, subbuf_size, num_subbuf, switch_timer_interval, read_timer_interval, stream_fds, nr_stream_fds, blocking_timeout); if (!handle) - return NULL; - lttng_chan = priv; + goto error; lttng_chan->handle = handle; lttng_chan->chan = shmp(handle, handle->chan); return lttng_chan; + +error: + free(lttng_chan); + return NULL; } static void lttng_channel_destroy(struct lttng_channel *chan) { channel_destroy(chan->chan, chan->handle, 1); + free(chan); } static diff --git a/liblttng-ust/lttng-ring-buffer-metadata-client.h b/liblttng-ust/lttng-ring-buffer-metadata-client.h index 2b2aae7d..956de9fd 100644 --- a/liblttng-ust/lttng-ring-buffer-metadata-client.h +++ b/liblttng-ust/lttng-ring-buffer-metadata-client.h @@ -200,33 +200,43 @@ struct lttng_channel *_channel_create(const char *name, const int *stream_fds, int nr_stream_fds, int64_t blocking_timeout) { - struct lttng_channel chan_priv_init; + struct lttng_ust_abi_channel_config chan_priv_init; struct lttng_ust_shm_handle *handle; struct lttng_channel *lttng_chan; - void *priv; + + lttng_chan = zmalloc(sizeof(struct lttng_channel)); + if (!lttng_chan) + return NULL; + memcpy(lttng_chan->uuid, uuid, LTTNG_UST_UUID_LEN); + lttng_chan->id = chan_id; memset(&chan_priv_init, 0, sizeof(chan_priv_init)); memcpy(chan_priv_init.uuid, uuid, LTTNG_UST_UUID_LEN); chan_priv_init.id = chan_id; + handle = channel_create(&client_config, name, - &priv, __alignof__(struct lttng_channel), + __alignof__(struct lttng_channel), sizeof(struct lttng_channel), &chan_priv_init, - buf_addr, subbuf_size, num_subbuf, + lttng_chan, buf_addr, subbuf_size, num_subbuf, switch_timer_interval, read_timer_interval, stream_fds, nr_stream_fds, blocking_timeout); if (!handle) - return NULL; - lttng_chan = priv; + goto error; lttng_chan->handle = handle; lttng_chan->chan = shmp(handle, handle->chan); return lttng_chan; + +error: + free(lttng_chan); + return NULL; } static void lttng_channel_destroy(struct lttng_channel *chan) { channel_destroy(chan->chan, chan->handle, 1); + free(chan); } static diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c index 12cb91f7..a1903451 100644 --- a/liblttng-ust/lttng-ust-abi.c +++ b/liblttng-ust/lttng-ust-abi.c @@ -448,6 +448,7 @@ int lttng_abi_map_channel(int session_objd, const char *chan_name; int chan_objd; struct lttng_ust_shm_handle *channel_handle; + struct lttng_ust_abi_channel_config *lttng_chan_config; struct lttng_channel *lttng_chan; struct lttng_ust_lib_ring_buffer_channel *chan; struct lttng_ust_lib_ring_buffer_config *config; @@ -475,6 +476,12 @@ int lttng_abi_map_channel(int session_objd, goto active; /* Refuse to add channel to active session */ } + lttng_chan = zmalloc(sizeof(struct lttng_channel)); + if (!lttng_chan) { + ret = -ENOMEM; + goto lttng_chan_error; + } + channel_handle = channel_handle_create(chan_data, len, wakeup_fd); if (!channel_handle) { ret = -EINVAL; @@ -489,8 +496,8 @@ int lttng_abi_map_channel(int session_objd, assert(chan); chan->handle = channel_handle; config = &chan->backend.config; - lttng_chan = channel_get_private(chan); - if (!lttng_chan) { + lttng_chan_config = channel_get_private_config(chan); + if (!lttng_chan_config) { ret = -EINVAL; goto alloc_error; } @@ -550,6 +557,10 @@ int lttng_abi_map_channel(int session_objd, lttng_chan->header_type = 0; lttng_chan->handle = channel_handle; lttng_chan->type = type; + /* Copy fields from lttng ust chan config. */ + lttng_chan->id = lttng_chan_config->id; + memcpy(lttng_chan->uuid, lttng_chan_config->uuid, LTTNG_UST_UUID_LEN); + channel_set_private(chan, lttng_chan); /* * We tolerate no failure path after channel creation. It will stay @@ -566,9 +577,12 @@ objd_error: notransport: alloc_error: channel_destroy(chan, channel_handle, 0); + free(lttng_chan); return ret; handle_error: + free(lttng_chan); +lttng_chan_error: active: invalid: return ret; diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h index e9f14fc4..bed7a85c 100644 --- a/liblttng-ust/ust-events-internal.h +++ b/liblttng-ust/ust-events-internal.h @@ -342,6 +342,32 @@ struct lttng_ust_channel_ops_private { struct lttng_ust_shm_handle *handle); }; +/* + * IMPORTANT: this structure is part of the ABI between the consumer + * daemon and the UST library within traced applications. Changing it + * breaks the UST communication protocol. + * + * TODO: remove unused fields on next UST communication protocol + * breaking update. + */ +struct lttng_ust_abi_channel_config { + void *unused1; + int unused2; + void *unused3; + void *unused4; + int unused5; + struct cds_list_head unused6; + void *unused7; + int unused8; + void *unused9; + + /* Channel ID */ + unsigned int id; + enum lttng_ust_abi_chan_type unused10; + unsigned char uuid[LTTNG_UST_UUID_LEN]; /* Trace session unique ID */ + int unused11:1; +}; + static inline struct lttng_ust_type_integer *lttng_ust_get_type_integer(struct lttng_ust_type_common *type) { diff --git a/libringbuffer/frontend.h b/libringbuffer/frontend.h index de924a0a..5e1ada15 100644 --- a/libringbuffer/frontend.h +++ b/libringbuffer/frontend.h @@ -34,20 +34,19 @@ * address mapping. It is used only by RING_BUFFER_STATIC configuration. It can * be set to NULL for other backends. * - * priv_data (output) is set to a pointer into a "priv_data_len"-sized - * memory area for client-specific data. This memory is managed by lib - * ring buffer. priv_data_align is the alignment required for the - * private data area. + * private data is a memory area for configuration data. This memory is + * managed by lib ring buffer. priv_data_align is the alignment required + * for the private data area. */ __attribute__((visibility("hidden"))) extern struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_lib_ring_buffer_config *config, const char *name, - void **priv_data, size_t priv_data_align, size_t priv_data_size, void *priv_data_init, + void *priv, void *buf_addr, size_t subbuf_size, size_t num_subbuf, unsigned int switch_timer_interval, diff --git a/libringbuffer/frontend_types.h b/libringbuffer/frontend_types.h index 01fdaccf..83564768 100644 --- a/libringbuffer/frontend_types.h +++ b/libringbuffer/frontend_types.h @@ -52,13 +52,14 @@ struct lttng_ust_lib_ring_buffer_channel { int read_timer_enabled; int finalized; /* Has channel been finalized */ - size_t priv_data_offset; + size_t priv_data_offset; /* Offset of private data channel config */ unsigned int nr_streams; /* Number of streams */ struct lttng_ust_shm_handle *handle; /* Extended options. */ union { struct { int32_t blocking_timeout_ms; + void *priv; /* Private data pointer. */ } s; char padding[RB_CHANNEL_PADDING]; } u; @@ -223,11 +224,23 @@ struct lttng_ust_lib_ring_buffer { } __attribute__((aligned(CAA_CACHE_LINE_SIZE))); static inline -void *channel_get_private(struct lttng_ust_lib_ring_buffer_channel *chan) +void *channel_get_private_config(struct lttng_ust_lib_ring_buffer_channel *chan) { return ((char *) chan) + chan->priv_data_offset; } +static inline +void *channel_get_private(struct lttng_ust_lib_ring_buffer_channel *chan) +{ + return chan->u.s.priv; +} + +static inline +void channel_set_private(struct lttng_ust_lib_ring_buffer_channel *chan, void *priv) +{ + chan->u.s.priv = priv; +} + #ifndef __rb_same_type #define __rb_same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) #endif diff --git a/libringbuffer/ring_buffer_frontend.c b/libringbuffer/ring_buffer_frontend.c index 23122865..7b276ad9 100644 --- a/libringbuffer/ring_buffer_frontend.c +++ b/libringbuffer/ring_buffer_frontend.c @@ -336,7 +336,7 @@ int lib_ring_buffer_create(struct lttng_ust_lib_ring_buffer *buf, struct lttng_ust_lib_ring_buffer_backend_subbuffer *wsb; struct lttng_ust_lib_ring_buffer_channel *shmp_chan; struct commit_counters_hot *cc_hot; - void *priv = channel_get_private(chan); + void *priv = channel_get_private_config(chan); size_t subbuf_header_size; uint64_t tsc; int ret; @@ -938,9 +938,10 @@ static void channel_free(struct lttng_ust_lib_ring_buffer_channel *chan, * channel_create - Create channel. * @config: ring buffer instance configuration * @name: name of the channel - * @priv_data: ring buffer client private data area pointer (output) - * @priv_data_size: length, in bytes, of the private data area. - * @priv_data_init: initialization data for private data. + * @priv_data_align: alignment, in bytes, of the private data area. (config) + * @priv_data_size: length, in bytes, of the private data area. (config) + * @priv_data_init: initialization data for private data. (config) + * @priv: local private data (memory owner by caller) * @buf_addr: pointer the the beginning of the preallocated buffer contiguous * address mapping. It is used only by RING_BUFFER_STATIC * configuration. It can be set to NULL for other backends. @@ -958,10 +959,10 @@ static void channel_free(struct lttng_ust_lib_ring_buffer_channel *chan, */ struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_lib_ring_buffer_config *config, const char *name, - void **priv_data, size_t priv_data_align, size_t priv_data_size, void *priv_data_init, + void *priv, void *buf_addr, size_t subbuf_size, size_t num_subbuf, unsigned int switch_timer_interval, unsigned int read_timer_interval, @@ -1035,6 +1036,8 @@ struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_lib_ring_buff /* space for private data */ if (priv_data_size) { + void *priv_config; + DECLARE_SHMP(void, priv_data_alloc); align_shm(shmobj, priv_data_align); @@ -1042,16 +1045,16 @@ struct lttng_ust_shm_handle *channel_create(const struct lttng_ust_lib_ring_buff set_shmp(priv_data_alloc, zalloc_shm(shmobj, priv_data_size)); if (!shmp(handle, priv_data_alloc)) goto error_append; - *priv_data = channel_get_private(chan); - memcpy(*priv_data, priv_data_init, priv_data_size); + priv_config = channel_get_private_config(chan); + memcpy(priv_config, priv_data_init, priv_data_size); } else { chan->priv_data_offset = -1; - if (priv_data) - *priv_data = NULL; } chan->u.s.blocking_timeout_ms = (int32_t) blocking_timeout_ms; + channel_set_private(chan, priv); + ret = channel_backend_init(&chan->backend, name, config, subbuf_size, num_subbuf, handle, stream_fds); @@ -1681,8 +1684,7 @@ void lib_ring_buffer_print_subbuffer_errors(struct lttng_ust_lib_ring_buffer *bu static void lib_ring_buffer_print_buffer_errors(struct lttng_ust_lib_ring_buffer *buf, struct lttng_ust_lib_ring_buffer_channel *chan, - void *priv, int cpu, - struct lttng_ust_shm_handle *handle) + int cpu, struct lttng_ust_shm_handle *handle) { const struct lttng_ust_lib_ring_buffer_config *config = &chan->backend.config; unsigned long write_offset, cons_offset; @@ -1715,7 +1717,6 @@ void lib_ring_buffer_print_errors(struct lttng_ust_lib_ring_buffer_channel *chan struct lttng_ust_shm_handle *handle) { const struct lttng_ust_lib_ring_buffer_config *config = &chan->backend.config; - void *priv = channel_get_private(chan); if (!strcmp(chan->backend.name, "relay-metadata-mmap")) { DBG("ring buffer %s: %lu records written, " @@ -1741,7 +1742,7 @@ void lib_ring_buffer_print_errors(struct lttng_ust_lib_ring_buffer_channel *chan v_read(config, &buf->records_lost_wrap), v_read(config, &buf->records_lost_big)); } - lib_ring_buffer_print_buffer_errors(buf, chan, priv, cpu, handle); + lib_ring_buffer_print_buffer_errors(buf, chan, cpu, handle); } /* -- 2.34.1