* Julien Desfossez ([email protected]) 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 <[email protected]>
Acked-by: Mathieu Desnoyers <[email protected]> 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 _______________________________________________ ltt-dev mailing list [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
