On Thu, Jul 20, 2017 at 10:22:13AM +0800, Anand Jain wrote: > >> @@ -1189,11 +1189,10 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page, > >> async_cow->locked_page = locked_page; > >> async_cow->start = start; > >> > >> - if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS && > >> - !btrfs_test_opt(fs_info, FORCE_COMPRESS)) > >> - cur_end = end; > >> - else > >> + if (inode_need_compress(inode)) > >> cur_end = min(end, start + SZ_512K - 1); > >> + else > >> + cur_end = end; > > > > The opencoded test should be cleaned up, however cow_file_range_async is > > called from run_delalloc_range if the inode_need_compress passes the > > 'inode_need_compress' condition. So at minimum, checking again here is > > redundant, > > David, no its not redundant. If the preceding SZ_512K block is not > compressible, then we would have already set the BTRFS_INODE_NOCOMPRESS > flag, which then we would straightaway go up to the end. Right, I missed that it's using 'start', that changes in the loop and that NOCOMPRESS can get set indirectly through the async write between the iterations. I'm still counting too many times where the expensive "should we compress" check can get triggerd. It's at least once before we go to cow_file_range_async and then again for each compressed range in compress_file_range. I think we can fix that by two modes of inode_need_compres or separate the lightweight and expensive checks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
