[ltt-dev] [LTTNG-TOOLS PATCH v3] Cleanup mmap consuming
David Goulet
david.goulet at polymtl.ca
Fri Aug 12 10:59:02 EDT 2011
Merged! Thanks
FIN
David
On 11-08-12 10:50 AM, Mathieu Desnoyers wrote:
> * 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
>>
>
More information about the lttng-dev
mailing list