From 14af8709e68d4f2d5bf905bf516e71307d2b1315 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 19 May 2010 17:49:56 -0400 Subject: [PATCH] liblttd: don't fill the page cache * Linus Torvalds (torvalds@linux-foundation.org) wrote: > > > On Wed, 19 May 2010, Mathieu Desnoyers wrote: > > > > Good point. This discard flag might do the trick and let us keep things simple. > > The major concern here is to keep the page cache disturbance relatively low. > > Which of new page allocation or stealing back the page has the lowest overhead > > would have to be determined with benchmarks. > > We could probably make it easier somehow to do the writeback and discard > thing, but I have had _very_ good experiences with even a rather trivial > file writer that basically used (iirc) 8MB windows, and the logic was very > trivial: > > - before writing a new 8M window, do "start writeback" > (SYNC_FILE_RANGE_WRITE) on the previous window, and do > a wait (SYNC_FILE_RANGE_WAIT_AFTER) on the window before that. > > in fact, in its simplest form, you can do it like this (this is from my > "overwrite disk images" program that I use on old disks): > > for (index = 0; index < max_index ;index++) { > if (write(fd, buffer, BUFSIZE) != BUFSIZE) > break; > /* This won't block, but will start writeout asynchronously */ > sync_file_range(fd, index*BUFSIZE, BUFSIZE, SYNC_FILE_RANGE_WRITE); > /* This does a blocking write-and-wait on any old ranges */ > if (index) > sync_file_range(fd, (index-1)*BUFSIZE, BUFSIZE, +SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE|SYNC_FILE_RANGE_WAIT_AFTER); > } > and even if you don't actually do a discard (maybe we should add a > SYNC_FILE_RANGE_DISCARD bit, right now you'd need to do a separate > fadvise(FADV_DONTNEED) to throw it out) the system behavior is pretty > nice, because the heavy writer gets good IO performance _and_ leaves only > easy-to-free pages around after itself. Great! I just implemented it in LTTng and it works very well ! A faced a small counter-intuitive fadvise behavior though. posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED); only seems to affect the parts of a file that already exist. So after each splice() that appends to the file, I have to call fadvise again. I would have expected the "0" len parameter to tell the kernel to apply the hint to the whole file, even parts that will be added in the future. I expect we have this behavior because fadvise() was initially made with read behavior in mind rather than write. For the records, I do a fadvice+async range write after each splice(). Also, after each subbuffer write, I do a blocking write-and-wait on all pages that are in the subbuffer prior to the one that has just been written, instead of using the fixed 8MB window. Signed-off-by: Mathieu Desnoyers --- liblttd/liblttd.c | 6 +++-- liblttd/liblttd.h | 3 +++ liblttd/liblttdvfs.c | 55 ++++++++++++++++++++++++++++++++++++-------- 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/liblttd/liblttd.c b/liblttd/liblttd.c index 91f3a27..2b187c8 100644 --- a/liblttd/liblttd.c +++ b/liblttd/liblttd.c @@ -109,6 +109,7 @@ int open_buffer_file(struct liblttd_instance *instance, char *filename, { int open_ret = 0; int ret = 0; + int fd; if (strncmp(filename, "flight-", sizeof("flight-")-1) != 0) { if (instance->dump_flight_only) { @@ -129,8 +130,9 @@ int open_buffer_file(struct liblttd_instance *instance, char *filename, ++instance->fd_pairs.num_pairs * sizeof(struct fd_pair)); /* Open the channel in read mode */ - instance->fd_pairs.pair[instance->fd_pairs.num_pairs-1].channel = - open(path_channel, O_RDONLY | O_NONBLOCK); + fd = open(path_channel, O_RDONLY | O_NONBLOCK); + instance->fd_pairs.pair[instance->fd_pairs.num_pairs-1].channel = fd; + if (instance->fd_pairs.pair[instance->fd_pairs.num_pairs-1].channel == -1) { perror(path_channel); instance->fd_pairs.num_pairs--; diff --git a/liblttd/liblttd.h b/liblttd/liblttd.h index 0b6b058..aac1e5d 100644 --- a/liblttd/liblttd.h +++ b/liblttd/liblttd.h @@ -27,6 +27,7 @@ #include #include +#include /** * struct fd_pair - Contains the data associated with the channel file @@ -39,6 +40,7 @@ * @mmap: Not used anymore. * @mutex: a mutex for internal library usage * @user_data: library user data + * @offset: write position in the output file descriptor (optional) */ struct fd_pair { int channel; @@ -47,6 +49,7 @@ struct fd_pair { void *mmap; pthread_mutex_t mutex; void *user_data; + off_t offset; }; struct channel_trace_fd { diff --git a/liblttd/liblttdvfs.c b/liblttd/liblttdvfs.c index 7b6ef0e..6e28c9b 100644 --- a/liblttd/liblttdvfs.c +++ b/liblttd/liblttdvfs.c @@ -35,7 +35,7 @@ #define _REENTRANT #define _GNU_SOURCE - +#define _XOPEN_SOURCE 600 #include #include #include @@ -73,6 +73,7 @@ int liblttdvfs_on_open_channel(struct liblttd_callbacks *data, struct fd_pair *p int ret; struct stat stat_buf; struct liblttdvfs_channel_data *channel_data; + off_t offset = 0; pair->user_data = malloc(sizeof(struct liblttdvfs_channel_data)); channel_data = pair->user_data; @@ -94,8 +95,8 @@ int liblttdvfs_on_open_channel(struct liblttd_callbacks *data, struct fd_pair *p open_ret = -1; goto end; } - ret = lseek(channel_data->trace, 0, SEEK_END); - if (ret < 0) { + offset = lseek(channel_data->trace, 0, SEEK_END); + if (offset < 0) { perror(callbacks_data->path_trace); open_ret = -1; close(channel_data->trace); @@ -115,9 +116,13 @@ int liblttdvfs_on_open_channel(struct liblttd_callbacks *data, struct fd_pair *p open_ret = -1; goto end; } + offset = 0; + } else { + perror("Channel output file open"); + open_ret = -1; + goto end; } } - end: return open_ret; @@ -157,6 +162,8 @@ int liblttdvfs_on_read_subbuffer(struct liblttd_callbacks *data, struct fd_pair { long ret; off_t offset = 0; + off_t orig_offset = pair->offset; + int outfd = ((struct liblttdvfs_channel_data *)(pair->user_data))->trace; struct liblttdvfs_data* callbacks_data = data->user_data; @@ -168,20 +175,48 @@ int liblttdvfs_on_read_subbuffer(struct liblttd_callbacks *data, struct fd_pair printf_verbose("splice chan to pipe ret %ld\n", ret); if (ret < 0) { perror("Error in relay splice"); - goto write_error; + goto write_end; } - ret = splice(thread_pipe[0], NULL, - ((struct liblttdvfs_channel_data *)(pair->user_data))->trace, + ret = splice(thread_pipe[0], NULL, outfd, NULL, ret, SPLICE_F_MOVE | SPLICE_F_MORE); printf_verbose("splice pipe to file %ld\n", ret); if (ret < 0) { perror("Error in file splice"); - goto write_error; + goto write_end; } len -= ret; - } + /* + * Give hints to the kernel about how we access the file: + * POSIX_FADV_DONTNEED : we won't re-access data in a near + * future after we write it. + * We need to call fadvise again after the file grows because + * the kernel does not seem to apply fadvise to non-existing + * parts of the file. + */ + ret = posix_fadvise(outfd, 0, 0, POSIX_FADV_DONTNEED); + if (ret != 0) { + perror("fadvise"); + /* Just a hint, not critical. Continue. */ + ret = 0; + } -write_error: + /* This won't block, but will start writeout asynchronously */ + sync_file_range(outfd, pair->offset, ret, + SYNC_FILE_RANGE_WRITE); + pair->offset += ret; + } +write_end: + /* + * This does a blocking write-and-wait on any page that belongs to the + * subbuffer prior to the one we just wrote. + */ + if (orig_offset >= pair->max_sb_size) + sync_file_range(outfd, orig_offset - pair->max_sb_size, + pair->max_sb_size, + SYNC_FILE_RANGE_WAIT_BEFORE + | SYNC_FILE_RANGE_WRITE + | SYNC_FILE_RANGE_WAIT_AFTER); + return ret; } -- 2.34.1