[ltt-dev] [LTTNG-TOOLS PATCH 3/3] Cleanup mmap consuming
Mathieu Desnoyers
compudj at krystal.dyndns.org
Fri Aug 5 11:42:07 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 the buffer is closing, we munmap it.
> Also removed the manual padding handling because the kernel already
> exposes a padded subbuffer.
>
> Signed-off-by: Julien Desfossez <julien.desfossez at polymtl.ca>
> ---
> liblttkconsumerd/liblttkconsumerd.c | 60 +++++++++++++++--------------------
> liblttkconsumerd/liblttkconsumerd.h | 2 +
> ltt-kconsumerd/ltt-kconsumerd.c | 2 +-
> 3 files changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/liblttkconsumerd/liblttkconsumerd.c b/liblttkconsumerd/liblttkconsumerd.c
> index 9d8cb00..81ea529 100644
> --- a/liblttkconsumerd/liblttkconsumerd.c
> +++ b/liblttkconsumerd/liblttkconsumerd.c
> @@ -141,6 +141,7 @@ 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) {
> @@ -148,6 +149,12 @@ static void kconsumerd_del_fd(struct kconsumerd_fd *lcf)
> if (lcf != NULL) {
> close(lcf->out_fd);
> close(lcf->consumerd_fd);
> + if (lcf->mmap_base != NULL) {
> + ret = munmap(lcf->mmap_base, lcf->mmap_len);
> + if (ret != 0) {
> + perror("munmap");
> + }
> + }
> free(lcf);
> lcf = NULL;
> }
> @@ -178,6 +185,8 @@ static int kconsumerd_add_fd(struct lttcomm_kconsumerd_msg *buf, int consumerd_f
> tmp_fd->consumerd_fd = consumerd_fd;
> tmp_fd->state = buf->state;
> tmp_fd->max_sb_size = buf->max_sb_size;
> + tmp_fd->mmap_base = NULL;
> + tmp_fd->mmap_len = 0;
> strncpy(tmp_fd->path_name, buf->path_name, PATH_MAX);
>
> /* Opening the tracefile in write mode */
> @@ -270,31 +279,29 @@ static int kconsumerd_update_poll_array(struct pollfd **pollfd,
> int kconsumerd_on_read_subbuffer_mmap(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);
> + if (kconsumerd_fd->mmap_base == NULL) {
> + /* get the len of the mmap region */
> + ret = kernctl_get_mmap_len(fd, &kconsumerd_fd->mmap_len);
> + if (ret != 0) {
> + ret = errno;
> + perror("kernctl_get_mmap_len");
> + goto end;
> + }
>
> - /* 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;
> + kconsumerd_fd->mmap_base = mmap(NULL, kconsumerd_fd->mmap_len,
> + PROT_READ, MAP_PRIVATE, fd, 0);
> + if (kconsumerd_fd->mmap_base == MAP_FAILED) {
> + perror("Error mmaping");
> + ret = -1;
> + goto end;
Although this works, I would really be tempted to move the
kernctl_get_mmap_len + mmap operations into the kconsumerd_add_fd()
function, because this is really at that point the initialization should
take place, rather than lazily in the read subbuffer operation. It would
then become much clearer when we get an error message upon
initialization error there rather than in the first read.
But it implies that you'd need to pass along some information about the
nature of the buffer (is it configured to be spliced or mmap'd)
directly to kconsumerd_add_fd, possible in the struct
lttcomm_kconsumerd_msg. Is that doable ?
Thanks,
Mathieu
> + }
> }
>
> /* get the offset inside the fd to mmap */
> @@ -305,15 +312,8 @@ int kconsumerd_on_read_subbuffer_mmap(struct kconsumerd_fd *kconsumerd_fd,
> 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) {
> @@ -327,14 +327,6 @@ int kconsumerd_on_read_subbuffer_mmap(struct kconsumerd_fd *kconsumerd_fd,
> 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.
> diff --git a/liblttkconsumerd/liblttkconsumerd.h b/liblttkconsumerd/liblttkconsumerd.h
> index f98621f..3de4c04 100644
> --- a/liblttkconsumerd/liblttkconsumerd.h
> +++ b/liblttkconsumerd/liblttkconsumerd.h
> @@ -55,6 +55,8 @@ 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;
> };
>
> /*
> diff --git a/ltt-kconsumerd/ltt-kconsumerd.c b/ltt-kconsumerd/ltt-kconsumerd.c
> index 61ed005..32d439a 100644
> --- a/ltt-kconsumerd/ltt-kconsumerd.c
> +++ b/ltt-kconsumerd/ltt-kconsumerd.c
> @@ -234,7 +234,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.");
> --
> 1.7.4.1
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list