On Tue, Sep 05, 2017 at 01:45:59PM -0600, Jens Axboe wrote: > On 09/05/2017 01:00 PM, Sitsofe Wheeler wrote: > > On 5 September 2017 at 15:58, Jens Axboe <axboe@xxxxxxxxx> wrote: > >> On 09/05/2017 08:54 AM, Tomohiro Kusumi wrote: > >>> I think I've made three fixes over the original two commits. > >>> > >>> 1. non O_DIRECT dio support -> committed -> now reverted > >>> 2. segfault on fio_memfree() -> committed -> now reverted > >>> 3. this one > >>> > >>> As far as I've seen this is it. > >> > >> Right, just checked (and reverted) and it's 5 changes all in > >> all for the change, not including the parent to this email, > >> which would have made it 6 in all. The original change from Weiping > >> clearly wasn't well tested or thought through, so it's better > >> to just kill it all and do it cleanly from scratch instead. > > > > Unfortunately it's been a problem magnetic with stacks of extra > > conditions turning up and corner cases to stumble over (see > > https://github.com/sitsofe/fio/commit/8fed4ccc87e39144643b1e32caddd882e6197e58 > > for yet another case that was going to need fixing). > > > >> That said, I'm not even convinced we need this change. Logically > >> it makes sense, but there's really nothing wrong with doing > >> a buffered layout + cache kill as we have been doing since > >> the dawn of time in fio. > > > > One of the benefits was that it stopped > > > > rm -f /tmp/fiofile; ./fio --loops=10 --filename /tmp/fiofile --bs=4k \ > > --size=1M --direct=1 --name=go > > > > reporting cached speeds on macOS (a platform where invalidation > > doesn't work). The sad thing is that the change wound up slowing > > layout right down because the I/O is potentially done in such small > > sizes. With the problems it attracted it need a re-think if it's to > > avoid attracting a never ending stream of workarounds. > > Slowing down layout for everyone is a much larger issue (and a > regression), whereas the lack of cache invalidation on a 2nd tier > platform is much less interesting. Of the two, I know what I'd pick. > > It'd be nice if we can get everything working nicely. Honestly, it's not > rocket science (at all) to use O_DIRECT for layouts. The performance > issue is the biggest hurdle, the only thing we can do there is use a > larger block size and hope it closes enough of the gap. > Hi Jens, How about add a slight check, open a temp file with O_DIRECT but pre-allocate using buffer io, like following: diff --git a/filesetup.c b/filesetup.c index b51ab35..7b86938 100644 --- a/filesetup.c +++ b/filesetup.c @@ -107,7 +107,7 @@ static void fallocate_file(struct thread_data *td, struct fio_file *f) */ static int extend_file(struct thread_data *td, struct fio_file *f) { - int new_layout = 0, unlink_file = 0, flags; + int new_layout = 0, unlink_file = 0, flags, tmp_fd; unsigned long long left; unsigned int bs; char *b = NULL; @@ -152,6 +152,16 @@ static int extend_file(struct thread_data *td, struct fio_file *f) #endif dprint(FD_FILE, "open file %s, flags %x\n", f->file_name, flags); + + if (td->o.odirect) { + tmp_fd = open(f->file_name, flags | OS_O_DIRECT, 0644); + if (tmp_fd < 0 && errno == EINVAL) + log_err("fio: seems like filesystem does not support " \ + "direct=1/buffered=0\n"); + if (tmp_fd < 0) + return 1; + close(tmp_fd); + } + f->fd = open(f->file_name, flags, 0644); if (f->fd < 0) { int err = errno; Thanks -- To unsubscribe from this list: send the line "unsubscribe fio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
