diff mbox series

[v2,24/87] trace-cmd library: Add local helper function for data compression

Message ID 20210729050959.12263-25-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Trace file version 7 | expand

Commit Message

Tzvetomir Stoyanov (VMware) July 29, 2021, 5:08 a.m. UTC
The newly added helper functions read data from a file and compress it,
before writing into the trace file. The trace data is comressed in
chunks, which are page aligned. A new local define is introduced:
  PAGES_IN_CHUNK
which can be used to tune how big a compression chunk is.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-output.c | 70 ++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 6 deletions(-)

Comments

Steven Rostedt Aug. 17, 2021, 2:53 p.m. UTC | #1
On Thu, 29 Jul 2021 08:08:56 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> The newly added helper functions read data from a file and compress it,
> before writing into the trace file. The trace data is comressed in
> chunks, which are page aligned. A new local define is introduced:
>   PAGES_IN_CHUNK
> which can be used to tune how big a compression chunk is.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-output.c | 70 ++++++++++++++++++++++++++++++++----
>  1 file changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
> index 6a44a99b..90625c4e 100644
> --- a/lib/trace-cmd/trace-output.c
> +++ b/lib/trace-cmd/trace-output.c
> @@ -285,18 +285,26 @@ static unsigned long get_size(const char *file)
>  	return size;
>  }
>  
> -static tsize_t copy_file_fd(struct tracecmd_output *handle, int fd)
> +static tsize_t copy_file_fd(struct tracecmd_output *handle, int fd, unsigned long long max)
>  {
> +	tsize_t rsize = 0;
>  	tsize_t size = 0;
>  	char buf[BUFSIZ];
>  	stsize_t r;
>  
>  	do {
> -		r = read(fd, buf, BUFSIZ);
> +		if (max > 0 && (max - size) < BUFSIZ)
> +			rsize = (max - size);
> +		else
> +			rsize = BUFSIZ;
> +
> +		r = read(fd, buf, rsize);
>  		if (r > 0) {
>  			size += r;
>  			if (do_write_check(handle, buf, r))
>  				return 0;
> +			if (max > 0 && size >= max)
> +				break;
>  		}
>  	} while (r > 0);

I think this would be a bit cleaner:

static tsize_t copy_file_fd(struct tracecmd_output *handle, int fd, unsigned long long max)
{
	tsize_t rsize = BUFSIZ;
	tsize_t size = 0;
	char buf[BUFSIZ];
	stsize_t r;

	do {
		if (max && rsize > max)
			rsize = max;

		r = read(fd, buf, rsize);
		if (r > 0) {
			size += r;
			if (do_write_check(handle, buf, r))
				return 0;
			if (max) {
				max -= r;
				if (!max)
					break;
			}
		}
	} while (r > 0);

	return size;
}


>  
> @@ -314,12 +322,62 @@ static tsize_t copy_file(struct tracecmd_output *handle,
>  		tracecmd_warning("Can't read '%s'", file);
>  		return 0;
>  	}
> -	size = copy_file_fd(handle, fd);
> +	size = copy_file_fd(handle, fd, 0);
>  	close(fd);
>  
>  	return size;
>  }
>  
> +#define PAGES_IN_CHUNK 10
> +__hidden unsigned long long out_copy_fd_compress(struct tracecmd_output *handle,

The above name is also hard to understand. "out_copy_fd_compress"?
Would "copy_out_fd_compress()" be better?

Also, why is it __hidden and not static. it's not used outside this file.

If it gets used outside this file in a follow up patch, please convert
it from static to __hidden then.

-- Steve


> +						 int fd, unsigned long long max,
> +						 unsigned long long *write_size)
> +{
> +	unsigned long long rsize = 0;
> +	unsigned long long wsize = 0;
> +	unsigned long long size;
> +	int ret;
> +
> +	if (handle->compress) {
> +		rsize = max;
> +		ret = tracecmd_compress_copy_from(handle->compress, fd,
> +						  PAGES_IN_CHUNK * handle->page_size,
> +						  &rsize, &wsize);
> +		if (ret < 0)
> +			return 0;
> +
> +		size = rsize;
> +		if (write_size)
> +			*write_size = wsize;
> +	} else {
> +		size = copy_file_fd(handle, fd, max);
> +		if (write_size)
> +			*write_size = size;
> +	}
> +
> +	return size;
> +}
> +
> +static tsize_t copy_file_compress(struct tracecmd_output *handle,
> +				  const char *file, unsigned long long *write_size)
> +{
> +	int ret;
> +	int fd;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		tracecmd_warning("Can't read '%s'", file);
> +		return 0;
> +	}
> +
> +	ret = out_copy_fd_compress(handle, fd, 0, write_size);
> +	if (!ret)
> +		tracecmd_warning("Can't compress '%s'", file);
> +
> +	close(fd);
> +	return ret;
> +}
> +
>  /*
>   * Finds the path to the debugfs/tracing
>   * Allocates the string and stores it.
> @@ -516,7 +574,7 @@ static int read_header_files(struct tracecmd_output *handle, bool compress)
>  	endian8 = convert_endian_8(handle, size);
>  	if (do_write_check(handle, &endian8, 8))
>  		goto out_close;
> -	check_size = copy_file_fd(handle, fd);
> +	check_size = copy_file_fd(handle, fd, 0);
>  	close(fd);
>  	if (size != check_size) {
>  		tracecmd_warning("wrong size for '%s' size=%lld read=%lld", path, size, check_size);
> @@ -542,7 +600,7 @@ static int read_header_files(struct tracecmd_output *handle, bool compress)
>  	endian8 = convert_endian_8(handle, size);
>  	if (do_write_check(handle, &endian8, 8))
>  		goto out_close;
> -	check_size = copy_file_fd(handle, fd);
> +	check_size = copy_file_fd(handle, fd, 0);
>  	close(fd);
>  	if (size != check_size) {
>  		tracecmd_warning("wrong size for '%s'", path);
> @@ -1984,7 +2042,7 @@ __hidden int out_write_cpu_data(struct tracecmd_output *handle,
>  		if (lseek64(data[i].fd, data[i].offset, SEEK_SET) == (off64_t)-1)
>  			goto out_free;
>  		if (data[i].size) {
> -			read_size = copy_file_fd(handle, data[i].fd);
> +			read_size = copy_file_fd(handle, data[i].fd, data[i].size);
>  			if (read_size != data_files[i].file_size) {
>  				errno = EINVAL;
>  				tracecmd_warning("did not match size of %lld to %lld",
diff mbox series

Patch

diff --git a/lib/trace-cmd/trace-output.c b/lib/trace-cmd/trace-output.c
index 6a44a99b..90625c4e 100644
--- a/lib/trace-cmd/trace-output.c
+++ b/lib/trace-cmd/trace-output.c
@@ -285,18 +285,26 @@  static unsigned long get_size(const char *file)
 	return size;
 }
 
-static tsize_t copy_file_fd(struct tracecmd_output *handle, int fd)
+static tsize_t copy_file_fd(struct tracecmd_output *handle, int fd, unsigned long long max)
 {
+	tsize_t rsize = 0;
 	tsize_t size = 0;
 	char buf[BUFSIZ];
 	stsize_t r;
 
 	do {
-		r = read(fd, buf, BUFSIZ);
+		if (max > 0 && (max - size) < BUFSIZ)
+			rsize = (max - size);
+		else
+			rsize = BUFSIZ;
+
+		r = read(fd, buf, rsize);
 		if (r > 0) {
 			size += r;
 			if (do_write_check(handle, buf, r))
 				return 0;
+			if (max > 0 && size >= max)
+				break;
 		}
 	} while (r > 0);
 
@@ -314,12 +322,62 @@  static tsize_t copy_file(struct tracecmd_output *handle,
 		tracecmd_warning("Can't read '%s'", file);
 		return 0;
 	}
-	size = copy_file_fd(handle, fd);
+	size = copy_file_fd(handle, fd, 0);
 	close(fd);
 
 	return size;
 }
 
+#define PAGES_IN_CHUNK 10
+__hidden unsigned long long out_copy_fd_compress(struct tracecmd_output *handle,
+						 int fd, unsigned long long max,
+						 unsigned long long *write_size)
+{
+	unsigned long long rsize = 0;
+	unsigned long long wsize = 0;
+	unsigned long long size;
+	int ret;
+
+	if (handle->compress) {
+		rsize = max;
+		ret = tracecmd_compress_copy_from(handle->compress, fd,
+						  PAGES_IN_CHUNK * handle->page_size,
+						  &rsize, &wsize);
+		if (ret < 0)
+			return 0;
+
+		size = rsize;
+		if (write_size)
+			*write_size = wsize;
+	} else {
+		size = copy_file_fd(handle, fd, max);
+		if (write_size)
+			*write_size = size;
+	}
+
+	return size;
+}
+
+static tsize_t copy_file_compress(struct tracecmd_output *handle,
+				  const char *file, unsigned long long *write_size)
+{
+	int ret;
+	int fd;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		tracecmd_warning("Can't read '%s'", file);
+		return 0;
+	}
+
+	ret = out_copy_fd_compress(handle, fd, 0, write_size);
+	if (!ret)
+		tracecmd_warning("Can't compress '%s'", file);
+
+	close(fd);
+	return ret;
+}
+
 /*
  * Finds the path to the debugfs/tracing
  * Allocates the string and stores it.
@@ -516,7 +574,7 @@  static int read_header_files(struct tracecmd_output *handle, bool compress)
 	endian8 = convert_endian_8(handle, size);
 	if (do_write_check(handle, &endian8, 8))
 		goto out_close;
-	check_size = copy_file_fd(handle, fd);
+	check_size = copy_file_fd(handle, fd, 0);
 	close(fd);
 	if (size != check_size) {
 		tracecmd_warning("wrong size for '%s' size=%lld read=%lld", path, size, check_size);
@@ -542,7 +600,7 @@  static int read_header_files(struct tracecmd_output *handle, bool compress)
 	endian8 = convert_endian_8(handle, size);
 	if (do_write_check(handle, &endian8, 8))
 		goto out_close;
-	check_size = copy_file_fd(handle, fd);
+	check_size = copy_file_fd(handle, fd, 0);
 	close(fd);
 	if (size != check_size) {
 		tracecmd_warning("wrong size for '%s'", path);
@@ -1984,7 +2042,7 @@  __hidden int out_write_cpu_data(struct tracecmd_output *handle,
 		if (lseek64(data[i].fd, data[i].offset, SEEK_SET) == (off64_t)-1)
 			goto out_free;
 		if (data[i].size) {
-			read_size = copy_file_fd(handle, data[i].fd);
+			read_size = copy_file_fd(handle, data[i].fd, data[i].size);
 			if (read_size != data_files[i].file_size) {
 				errno = EINVAL;
 				tracecmd_warning("did not match size of %lld to %lld",