On Mon, Jun 24, 2019 at 09:17:20AM -0700, Darrick J. Wong wrote: > On Mon, Jun 24, 2019 at 07:52:48AM +0200, Christoph Hellwig wrote: > > We have historically decided that we want to preallocate the xfs_trans > > structure at writeback time so that we don't have to allocate on in > > the I/O completion handler. But we treat unwrittent extent and COW > > fork conversions different already, which proves that the transaction > > allocations in the end I/O handler are not a problem. Removing the > > preallocation gets rid of a lot of corner case code, and also ensures > > we only allocate one and log a transaction when actually required, > > as the ioend merging can reduce the number of actual i_size updates > > significantly. > > That's what I thought when I wrote the ioend merging patches, but IIRC > Dave objected on the grounds that most file writes are trivial file > extending writes and therefore we should leave this alone to avoid > slowing down the ioend path even if it came at a cost of cancelling a > lot of empty transactions. The issue is stuff like extracting a tarball, where we might write a hundred thousand files and they are all written in a single IO. i.e. there is no IO completion merging at all. > I wasn't 100% convinced it mattered but ran out of time in the > development window and never got around to researching if it made any > difference. Yeah, it's not all that simple :/ In these cases, we always have to allocate a transaction for every file being written. If we do it before we submit the IO, then all the transactions are allocated from the single writeback context. If we don't have log space, data writeback pauses while the tail of the AIL is pushed, metadata writeback occurs, and then the transaction allocation for data writeback is woken, and data writeback submission continues. It's fairly orderly, and we don't end up trying to write back data while we are doing bulk metadata flushing from the AIL. If we delay the transaction allocation to the ioend context and we are low on log space, we end up blocking a kworker on a transaction allocation which then has to wait for metadata writeback. The kworker infrastructure will then issue the next ioend work, which then blocks on transaction allocation. Delayed allocation can cause thousands of small file IOs to be inflight at the same time due to intra-file contiguous allocation and IO merging in the block layer, hence we can have thousands of individual IO completions that require transaction allocation to be completed. > So, uh, how much of a hit do we take for having to allocate a > transaction for a file size extension? Particularly since we can > combine those things now? Unless we are out of log space, the transaction allocation and free should be largely uncontended and so it's just a small amount of CPU usage. i.e it's a slab allocation/free and then lockless space reservation/free. If we are out of log space, then we sleep waiting for space - the issue really comes down to where it is better to sleep in that case.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
