On Wed 07-02-18 09:51:02, Chris Mason wrote: > On 7 Feb 2018, at 5:32, Jan Kara wrote: > > On Thu 25-01-18 08:41:58, Chris Mason wrote: > > > > > > With ext4, the data=ordered IO done during transaction commits makes > > > priority inversions that I don't see a way around. It's > > > dramatically better > > > than ext3, but still happens enough that we can't enforce IO limits > > > at all. > > > It really only takes one low prio IO to sneak into kjournald's list > > > to wreck > > > everything. > > > > AFAIU we could do a similar thing like what Tejun implemented for btrfs > > metadata where the submitter can override blkcg to which the IO is > > accounted. In ext4's case if kjournald is doing the writeback, it would > > get > > accounted to the root blkcg. It will allow containers to somewhat > > violate > > the bounds set to their blkcg but the priority inversion should be rarer > > - > > sadly we cannot easily make it completely go away as if the original > > process > > not only attaches the inode to the transaction but also submits the data > > blocks with low priority, transaction commit still has to wait for this > > IO to > > complete so the whole commit will be still blocked. > > Yeah, I think this was the problem we hit. balance_dirty_pages and friends > will trigger low priority write back, and if kjournald ends up waiting on > that, we're out of luck. > > > > > So probably a better fix would be to introduce another data journalling > > mode for ext4 where we'd unconditionally use unwritten extents for data > > writeback. We actually have it implemented in ext4 hidden behind > > 'dioread_nolock' mount option but it needs more polishing and possibly > > testing. > > I wonder how that compares in performance to my old data=guarded idea. I > think a better step one might be to add tracepoints when blocks are added to > the ordered list, so we can better understand if we're adding them in error. > It felt like it was happening more often than it should. In ext4 / jbd2 this mechanism is actually different from ext3. We don't track individual blocks in ordered list anymore, we just track inodes and flush all mapped & dirty pages from those inodes when doing transaction commit. It does potentially imply more work but the locking imposed by the original jbd scheme was incompatible with reasonably efficient writeback and generally very special data writeback path causing various troubles. We do take care to add inode to a transaction only when we allocate new block for it so in this sence we shouldn't ever be adding them "in error". You are right that we could optimize this by doing a special handling for writeback of blocks beyond end of file (which is what data=guarded was about if I remember right) however with delayed allocation you then have to be careful not to flush new inode size to disk before flushing the data blocks so it gets more complicated. But creating unwritten extents and converting them to written on IO completion certainly has non-negligible cost (that's why this is not the default yet) so possibly the complication is worth it. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
