[ltt-dev] [LTTNG-TOOLS PATCH v3] Cleanup mmap consuming

Julien Desfossez julien.desfossez at polymtl.ca
Fri Aug 12 10:45:28 EDT 2011


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>
---
 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