Re: [PATCH] btrfs: make btrfs_dirty_inode() to always reserve metadata space
From: Nikolay Borisov <hidden>
Date: 2021-01-12 12:20:20
On 8.01.21 г. 7:36 ч., Qu Wenruo wrote:
There are several qgroup flush related bugs fixed recently, all of them are caused by the fact that we can trigger qgroup metadata space reservation holding a transaction handle. Thankfully the only situation to trigger above reservation is btrfs_dirty_inode(). Currently btrfs_dirty_inode() will try join transactio first, then update the inode. If btrfs_update_inode() fails with -ENOSPC, then it retry to start transaction to reserve metadata space. This not only forces us to reserve metadata space with a transaction handle hold, but can't handle other errors like -EDQUOT. This patch will make btrfs_dirty_inode() to call btrfs_start_transaction() directly without first try joining then starting, so that in try_flush_qgroup() we won't hold a trans handle. This will slow down btrfs_dirty_inode() but my fstests doesn't show too much different for most test cases, thus it may be worthy to skip such performance "optimization".
btrfs_dirty_inode is called everytime file_update_time is called or atime is updated. Given the default mount option is to use lazy a_time I'd say we needn't worry about it. file_update_time, however, is called within the write path so this change potentially makes file write more expensive. So instead of testing with fstests it needs proper benchmarking with fio. <snip>