[ltt-dev] [LTTNG-TOOLS PATCH v3] Cleanup mmap consuming
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Fri Aug 12 10:50:17 EDT 2011
* Julien Desfossez (julien.desfossez at polymtl.ca) wrote:
> Only one mmap call per buffer, once the first mmap is done, we use the
> offset to know where to start consuming the data.
> When we receive the FD and its output mode is mmap, we mmap the buffer,
> when it is closing, we munmap it.
> Removed the manual padding handling because the kernel already exposes a
> padded subbuffer.
> Also updated the check of the return code of
> kconsumerd_consumerd_recv_fd to be consistent with the rest of the code,
> we are in error when it's < 0 not <= 0.
>
> Signed-off-by: Julien Desfossez <julien.desfossez at polymtl.ca>
Acked-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Thanks!
Mathieu
> ---
> include/lttng-sessiond-comm.h | 1 +
> liblttkconsumerd/lttkconsumerd.c | 69 +++++++++++++++++---------------------
> liblttkconsumerd/lttkconsumerd.h | 3 ++
> ltt-kconsumerd/ltt-kconsumerd.c | 2 +-
> ltt-sessiond/main.c | 2 +
> 5 files changed, 38 insertions(+), 39 deletions(-)
>
> diff --git a/include/lttng-sessiond-comm.h b/include/lttng-sessiond-comm.h
> index be903a6..f57b0cd 100644
> --- a/include/lttng-sessiond-comm.h
> +++ b/include/lttng-sessiond-comm.h
> @@ -193,6 +193,7 @@ struct lttcomm_kconsumerd_msg {
> int fd;
> uint32_t state; /* enum lttcomm_kconsumerd_fd_state */
> unsigned long max_sb_size; /* the subbuffer size for this channel */
> + enum lttng_event_output output; /* use splice or mmap to consume this fd */
> };
>
> extern int lttcomm_create_unix_sock(const char *pathname);
> diff --git a/liblttkconsumerd/lttkconsumerd.c b/liblttkconsumerd/lttkconsumerd.c
> index d4908d1..29f1f95 100644
> --- a/liblttkconsumerd/lttkconsumerd.c
> +++ b/liblttkconsumerd/lttkconsumerd.c
> @@ -127,11 +127,18 @@ static int kconsumerd_find_session_fd(int fd)
> */
> static void kconsumerd_del_fd(struct kconsumerd_fd *lcf)
> {
> + int ret;
> pthread_mutex_lock(&kconsumerd_data.lock);
> cds_list_del(&lcf->list);
> if (kconsumerd_data.fds_count > 0) {
> kconsumerd_data.fds_count--;
> if (lcf != NULL) {
> + if (lcf->mmap_base != NULL) {
> + ret = munmap(lcf->mmap_base, lcf->mmap_len);
> + if (ret != 0) {
> + perror("munmap");
> + }
> + }
> if (lcf->out_fd != 0) {
> close(lcf->out_fd);
> }
> @@ -168,6 +175,9 @@ static int kconsumerd_add_fd(struct lttcomm_kconsumerd_msg *buf, int consumerd_f
> tmp_fd->max_sb_size = buf->max_sb_size;
> tmp_fd->out_fd = 0;
> tmp_fd->out_fd_offset = 0;
> + tmp_fd->mmap_len = 0;
> + tmp_fd->mmap_base = NULL;
> + tmp_fd->output = buf->output;
> strncpy(tmp_fd->path_name, buf->path_name, PATH_MAX);
> tmp_fd->path_name[PATH_MAX - 1] = '\0';
>
> @@ -185,6 +195,24 @@ static int kconsumerd_add_fd(struct lttcomm_kconsumerd_msg *buf, int consumerd_f
> tmp_fd->sessiond_fd, tmp_fd->consumerd_fd, tmp_fd->out_fd);
> }
>
> + if (tmp_fd->output == LTTNG_EVENT_MMAP) {
> + /* get the len of the mmap region */
> + ret = kernctl_get_mmap_len(tmp_fd->consumerd_fd, &tmp_fd->mmap_len);
> + if (ret != 0) {
> + ret = errno;
> + perror("kernctl_get_mmap_len");
> + goto end;
> + }
> +
> + tmp_fd->mmap_base = mmap(NULL, tmp_fd->mmap_len,
> + PROT_READ, MAP_PRIVATE, tmp_fd->consumerd_fd, 0);
> + if (tmp_fd->mmap_base == MAP_FAILED) {
> + perror("Error mmaping");
> + ret = -1;
> + goto end;
> + }
> + }
> +
> cds_list_add(&tmp_fd->list, &kconsumerd_data.fd_list.head);
> kconsumerd_data.fds_count++;
> kconsumerd_data.need_update = 1;
> @@ -259,33 +287,13 @@ static int kconsumerd_update_poll_array(struct kconsumerd_local_data *ctx,
> int kconsumerd_on_read_subbuffer_mmap(struct kconsumerd_local_data *ctx,
> struct kconsumerd_fd *kconsumerd_fd, unsigned long len)
> {
> - unsigned long mmap_len, mmap_offset, padded_len, padding_len;
> - char *mmap_base;
> + unsigned long mmap_offset;
> char *padding = NULL;
> long ret = 0;
> off_t orig_offset = kconsumerd_fd->out_fd_offset;
> int fd = kconsumerd_fd->consumerd_fd;
> int outfd = kconsumerd_fd->out_fd;
>
> - /* get the padded subbuffer size to know the padding required */
> - ret = kernctl_get_padded_subbuf_size(fd, &padded_len);
> - if (ret != 0) {
> - ret = errno;
> - perror("kernctl_get_padded_subbuf_size");
> - goto end;
> - }
> - padding_len = padded_len - len;
> - padding = malloc(padding_len * sizeof(char));
> - memset(padding, '\0', padding_len);
> -
> - /* get the len of the mmap region */
> - ret = kernctl_get_mmap_len(fd, &mmap_len);
> - if (ret != 0) {
> - ret = errno;
> - perror("kernctl_get_mmap_len");
> - goto end;
> - }
> -
> /* get the offset inside the fd to mmap */
> ret = kernctl_get_mmap_read_offset(fd, &mmap_offset);
> if (ret != 0) {
> @@ -294,15 +302,8 @@ int kconsumerd_on_read_subbuffer_mmap(struct kconsumerd_local_data *ctx,
> goto end;
> }
>
> - mmap_base = mmap(NULL, mmap_len, PROT_READ, MAP_PRIVATE, fd, mmap_offset);
> - if (mmap_base == MAP_FAILED) {
> - perror("Error mmaping");
> - ret = -1;
> - goto end;
> - }
> -
> while (len > 0) {
> - ret = write(outfd, mmap_base, len);
> + ret = write(outfd, kconsumerd_fd->mmap_base + mmap_offset, len);
> if (ret >= len) {
> len = 0;
> } else if (ret < 0) {
> @@ -316,14 +317,6 @@ int kconsumerd_on_read_subbuffer_mmap(struct kconsumerd_local_data *ctx,
> kconsumerd_fd->out_fd_offset += ret;
> }
>
> - /* once all the data is written, write the padding to disk */
> - ret = write(outfd, padding, padding_len);
> - if (ret < 0) {
> - ret = errno;
> - perror("Error writing padding to file");
> - goto end;
> - }
> -
> /*
> * This does a blocking write-and-wait on any page that belongs to the
> * subbuffer prior to the one we just wrote.
> @@ -914,7 +907,7 @@ void *kconsumerd_thread_receive_fds(void *data)
> /* we received a command to add or update fds */
> ret = kconsumerd_consumerd_recv_fd(ctx, sock, kconsumerd_sockpoll,
> tmp.payload_size, tmp.cmd_type);
> - if (ret <= 0) {
> + if (ret < 0) {
> ERR("Receiving the FD, exiting");
> goto end;
> }
> diff --git a/liblttkconsumerd/lttkconsumerd.h b/liblttkconsumerd/lttkconsumerd.h
> index 10e4a55..1dd85d5 100644
> --- a/liblttkconsumerd/lttkconsumerd.h
> +++ b/liblttkconsumerd/lttkconsumerd.h
> @@ -55,6 +55,9 @@ struct kconsumerd_fd {
> char path_name[PATH_MAX]; /* tracefile name */
> enum kconsumerd_fd_state state;
> unsigned long max_sb_size; /* the subbuffer size for this channel */
> + void *mmap_base;
> + size_t mmap_len;
> + enum lttng_event_output output; /* splice or mmap */
> };
>
> struct kconsumerd_local_data {
> diff --git a/ltt-kconsumerd/ltt-kconsumerd.c b/ltt-kconsumerd/ltt-kconsumerd.c
> index 64c5ccf..ceaee71 100644
> --- a/ltt-kconsumerd/ltt-kconsumerd.c
> +++ b/ltt-kconsumerd/ltt-kconsumerd.c
> @@ -237,7 +237,7 @@ static int read_subbuffer(struct kconsumerd_fd *kconsumerd_fd)
> break;
> case LTTNG_EVENT_MMAP:
> /* read the used subbuffer size */
> - err = kernctl_get_subbuf_size(infd, &len);
> + err = kernctl_get_padded_subbuf_size(infd, &len);
> if (err != 0) {
> ret = errno;
> perror("Getting sub-buffer len failed.");
> diff --git a/ltt-sessiond/main.c b/ltt-sessiond/main.c
> index b1dbc8f..746cdd0 100644
> --- a/ltt-sessiond/main.c
> +++ b/ltt-sessiond/main.c
> @@ -288,6 +288,7 @@ static int send_kconsumerd_channel_fds(int sock, struct ltt_kernel_channel *chan
> lkm.fd = stream->fd;
> lkm.state = stream->state;
> lkm.max_sb_size = channel->channel->attr.subbuf_size;
> + lkm.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
> strncpy(lkm.path_name, stream->pathname, PATH_MAX);
> lkm.path_name[PATH_MAX - 1] = '\0';
>
> @@ -338,6 +339,7 @@ static int send_kconsumerd_fds(int sock, struct ltt_kernel_session *session)
> lkm.fd = session->metadata_stream_fd;
> lkm.state = ACTIVE_FD;
> lkm.max_sb_size = session->metadata->conf->attr.subbuf_size;
> + lkm.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
> strncpy(lkm.path_name, session->metadata->pathname, PATH_MAX);
> lkm.path_name[PATH_MAX - 1] = '\0';
>
> --
> 1.7.4.1
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list