On 1/4/21 12:23 PM, Filipe Manana wrote:
On Mon, Jan 4, 2021 at 5:06 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:This is a revert for 38d715f494f2 ("btrfs: use btrfs_start_delalloc_roots in shrink_delalloc"). A user reported a problem where performance was significantly worse with this patch applied. The problem needs to be fixed with proper pre-flushing, and changes to how we deal with the work queues for the inodes. However that work is much more complicated than is acceptable for stable, and simply reverting this patch fixes the problem. The original patch was a cleanup of the code, so it's fine to revert it. My numbers for the original reported test, which was untarring a copy of the firefox sources, are as follows 5.9 0m54.258s 5.10 1m26.212s Fix 0m35.038s cc: stable@xxxxxxxxxxxxxxx # 5.10 Reported-by: René Rebe <rene@xxxxxxxxxxxx> Fixes: 38d715f494f2 ("btrfs: use btrfs_start_delalloc_roots in shrink_delalloc") Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx> --- Dave, this is ontop of linus's branch, because we've changed the arguments for btrfs_start_delalloc_roots in misc-next, and this needs to go back to 5.10 ASAP. I can send a misc-next version if you want to have it there as well while we're waiting for it to go into linus's tree, just let me know.Adding this to stable releases will also make the following fix not work on stable releases: https://lore.kernel.org/linux-btrfs/39c2a60aa682f69f9823f51aa119d37ef4b9f834.1606909923.git.fdmanana@xxxxxxxx/ Since now the async reclaim task can trigger writeback through writeback_inodes_sb_nr() and not only through btrfs_start_delalloc_roots(). Other than changing that patch to make extent_write_cache_pages() do nothing when the inode has the bit BTRFS_INODE_NO_DELALLOC_FLUSH set, I'm not seeing other simple ways to do it.
Hmmm shit, ok let me see if I can make the perf regression go away while still using btrfs_start_delalloc_roots(). Thanks,
Josef
