On 2019/7/8 下午9:07, Nikolay Borisov wrote: > > > 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. Makes sense. I'm OK to drop it. Thanks, Qu > > <snip> >
