On 8.07.19 г. 15:50 ч., Qu Wenruo wrote: > > > On 2019/7/8 下午6:43, Nikolay Borisov wrote: >> >> >> On 8.07.19 г. 10:33 ч., Qu Wenruo wrote: >>> In write_one_cache_group() we always do COW to update BLOCK_GROUP_ITEM. >>> However under a lot of cases, the cache->item is not changed at all. >>> >>> E.g: >>> Transaction 123, block group [1M, 1M + 16M) >>> >>> tree block 1M + 0 get freed >>> tree block 1M + 16K get allocated. >>> >>> Transaction 123 get committed. >>> >>> In this case, used space of block group [1M, 1M + 16M) doesn't changed >>> at all, thus we don't need to do COW to update block group item. >>> >>> This patch will make write_one_cache_group() to do a read-only search >>> first, then do COW if we really need to update block group item. >>> >>> This should reduce the btrfs_write_dirty_block_groups() and >>> btrfs_run_delayed_refs() loop introduced in previous commit. >>> >>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx> >> >> I'm not sure how effective this is going to be > > The effectiveness is indeed low. > > For btrfs/013 test case, 64K page size, it reduces total number of > delayed refs by less than 2% (5/300+) > > And similar result for total number of dirty block groups. > >> and isn't this premature >> optimization, have you done any measurements? > > For the optimization part, I'd say it should be pretty safe. > It just really skips unnecessary CoW. > > The only downside to me is the extra tree search, thus killing the > "optimization" part. > If that's the case then I'd rather see the 2nd patch dropped. It adds more code for no gain. <snip>
