On Tue, Jul 21, 2009 at 6:27 PM, Curt Wohlgemuth<curtw@xxxxxxxxxx> wrote: > I spent a bit of time looking at this today. > > On Fri, Jun 12, 2009 at 10:33 AM, Theodore Tso<tytso@xxxxxxx> wrote: >> On Fri, Jun 12, 2009 at 06:01:12PM +0530, Aneesh Kumar K.V wrote: >>> Hi, >>> >>> I noticed yesterday that a write to fallocate >>> space via directIO results in fallback to buffer_IO. ie the userspace >>> pages get copied to the page cache and then call a sync. >>> >>> I guess this defeat the purpose of using directIO. May be we should >>> consider this a high priority bug. > > My simple experiment -- without a journal -- shows that you're > observation is correct. *Except* if FALLOC_FL_KEEP_SIZE is used in > the fallocate() call, in which case the page cache is *not* used. > > Pseudo-code example: > > open(O_DIRECT) > fallocate(mode, 512MB) > while (! written 100MB) > write(64K) > close() > > If mode == FALLOC_FL_KEEP_SIZE, then no page cache is used. > Otherwise, we *do* go through the page cache. > > It comes down to the fact that, since the i_size is not updated with > KEEP_SIZE, then ext4_get_block() is called with create = 1, since the > block that's needed is "beyond" the file end. Ted, given your concerns over the performance impact of updating the extents during direct I/O writes, it would seem that the fact that when KEEP_SIZE is specified we do the DMA (and don't go through the page cache) would be a problem/bug. At least, it seems that the performance issue is the same regardless of whether KEEP_SIZE is used on the fallocate or not: in both we're dealing with an uninitialized extent. Do you agree? I'm exploring (a) what this performance penalty is for the journal commit; and (b) can we at least avoid the page cache if your conditions above (no journal commit; no new extent blocks) are met. Curt > >> >> I agree that many of users of fallocate() feature (i.e. databases) are >> going to consider this to be a major misfeature. > >> >> There's going to be a major performance hit though --- O_DIRECT is >> supposed to be synchronous if all of the alignment requirements are >> met, which means that by the time the write(2) system call returns, >> the data is guaranteed to be on disk. But if we need to manipulate >> the extent tree to indicate that the block is now in use (so the data >> is actually accessible), do we force a synchronous journal commit or >> not? If we don't, then a crash right after an O_DIRECT right into an >> uninitialized region will cause the data to be "lost" (or at least, >> unavailable via the read/write system call). If we do, then the first >> write into uninitialized block will cause a synchronous journal commit >> that will be Slow And Painful, and it might destroy most of the >> performance benefits that might tempt an enterprise database client to >> use fallocate() in the first place. >> >> I wonder how XFS deals with this case? It's a problem that is going >> to hit any journalled filesystem that wants to support fallocate() and >> direct I/O. >> >> One thing I can think of potentially doing is to check to see if the >> extent tree block has already been journalled, and if it is not >> currently involved the current transaction or the previous committing >> transaction, *and* if there is space in the extent tree to mark the >> current unitialized block as initialized (i.e., if the extent needs to >> be split, there is sufficient space so we don't have to allocate a new >> leaf block for the extent tree), we could update the leaf block in >> place and then synchronously write it out, and thus avoid needing to >> do a synchronous journal commit. > > In my example above, when KEEP_SIZE is used, it appears that > converting the uninit extent to initialized never failed. I haven't > waded through ext4_ext_convert_to_initialized() to see how it might > fail, and tried to get it to do so. > > It would be interesting to see if making this work -- having the > blocks allocated and the buffer mapped -- for O_DIRECT writes in the > absence of a journal, at least, would be feasible. It would certainly > be useful, to us at least. > > Thanks, > Curt > >> >> In any case, adding this support is going to be non-trivial. If >> someone has time to work on it in the next 2-3 weeks or so, I can push >> it to Linus as a bug fix --- but I'm concerned the fixing this may be >> tricky enough (and the patch invasive enough) that it might be >> challenging to get this fixed in time for 2.6.31. >> >> - Ted >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
