Skip to contentSkip to conversation

Preemptive flushing fixes

PATCH In review Josef Bacik 3 participants 5 messages Aug 11, 2021 – Aug 17, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Preemptive flushing fixes

Hello,

I thought I had fixed the preemptive flushing burning CPU's problem with my previous set of fixes, but I was wrong. However those tracepoints gave me the information I needed to fix the problem properly. The first patch

btrfs: reduce the preemptive flushing threshold to 90%

can go back to stable and make its way into the distros to stop the pain for the current users having problems. The second patch augments the fix with a little less of a strong hammer.

The problem is for very full file systems on slower disks will end up with a very small threshold to start preemptive flushing. We were relying on sanity checks to bail out ahead of time, however they were not strong enough. These problematic cases existed in the short area where there was enough space to operate without needing to do synchronous flushing, but not enough space to avoid flushing all of the time.

The fix is to adjust the sanity checks to something more reasonable to account for these cases and avoid spinning doing preemptive flushing constantly. Thanks,

Josef

Josef Bacik (2):
btrfs: reduce the preemptive flushing threshold to 90%
btrfs: do not do preemptive flushing if the majority is global rsv

 fs/btrfs/space-info.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
Signature · 1 line
2.26.3
#1 lore raw

PATCH 1/2 btrfs: reduce the preemptive flushing threshold to 90%

The preemptive flushing code was added in order to avoid needing to synchronously wait for ENOSPC flushing to recover space. Once we're almost full however we can essentially flush constantly. We were using 98% as a threshold to determine if we were simply full, however in practice this is a really high bar to hit. For example reports of systems running into this problem had around 94% usage and thus continued to flush. Fix this by lowering the threshold to 90%, which is a more sane value, especially for smaller file systems.

Fixes 576fa34830af ("btrfs: improve preemptive background space flushing")
---
 fs/btrfs/space-info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/space-info.c +1 -1
index d9c8d738678f..ddb4878e94df 100644
@@ -733,7 +733,7 @@ static bool need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
 {
 	u64 global_rsv_size = fs_info->global_block_rsv.reserved;
 	u64 ordered, delalloc;
-	u64 thresh = div_factor_fine(space_info->total_bytes, 98);
+	u64 thresh = div_factor(space_info->total_bytes, 9);
 	u64 used;
 
 	/* If we're just plain full then async reclaim just slows us down. */
Signature · 1 line
2.26.3
#2 lore raw

PATCH 2/2 btrfs: do not do preemptive flushing if the majority is global rsv

In reply to Josef Bacik

A common characteristic of the bug report where preemptive flushing was going full tilt was the fact that the vast majority of the free metadata space was used up by the global reserve. The hard 90% threshold would cover the majority of these cases, but to be even smarter we should take into account how much of the outstanding reservations are covered by the global block reserve. If the global block reserve accounts for the vast majority of outstanding reservations, skip preemptive flushing, as it will likely just cause churn and pain.

---
 fs/btrfs/space-info.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
fs/btrfs/space-info.c +14 -0
index ddb4878e94df..2fce15d58b55 100644
@@ -741,6 +741,20 @@ static bool need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
 	     global_rsv_size) >= thresh)
 		return false;
 
+	used = space_info->bytes_may_use + space_info->bytes_pinned;
+
+	/* The total reservation belongs to the global rsv, don't flush. */
+	if (global_rsv_size >= used)
+		return false;
+
+	/*
+	 * 128m is 1/4 of the maximum global rsv size.  If we have less than
+	 * that devoted to other reservations then there's no sense in flushing,
+	 * we don't have a lot of things that need flushing.
+	 */
+	if ((used - global_rsv_size) <= SZ_128M)
+		return false;
+
 	/*
 	 * We have tickets queued, bail so we don't compete with the async
 	 * flushers.
Signature · 1 line
2.26.3
#3 lore raw

Re: [PATCH 2/2] btrfs: do not do preemptive flushing if the majority is global rsv

Quoted 20 lines · Josef Bacik

A common characteristic of the bug report where preemptive flushing was going full tilt was the fact that the vast majority of the free metadata space was used up by the global reserve. The hard 90% threshold would cover the majority of these cases, but to be even smarter we should take into account how much of the outstanding reservations are covered by the global block reserve. If the global block reserve accounts for the vast majority of outstanding reservations, skip preemptive flushing, as it will likely just cause churn and pain.

---
 fs/btrfs/space-info.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
fs/btrfs/space-info.c +0 -0
index ddb4878e94df..2fce15d58b55 100644

fs/btrfs/space-info.c static bool need_preemptive_reclaim(struct btrfs_fs_info *fs_info, · line 744

Quoted 4 lines
fs/btrfs/space-info.c +1 -0
@@ -741,6 +741,20 @@ static bool need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
 	     global_rsv_size) >= thresh)
 		return false;
 
+	used = space_info->bytes_may_use + space_info->bytes_pinned;

But global_rsv_size is accounted entirely in bytes_may_use (per btrfs_update_global_block_rsv logic), so why add bytes_pinned?

Quoted 17 lines
fs/btrfs/space-info.c +13 -0
+
+	/* The total reservation belongs to the global rsv, don't flush. */
+	if (global_rsv_size >= used)
+		return false;
+
+	/*
+	 * 128m is 1/4 of the maximum global rsv size.  If we have less than
+	 * that devoted to other reservations then there's no sense in flushing,
+	 * we don't have a lot of things that need flushing.
+	 */
+	if ((used - global_rsv_size) <= SZ_128M)
+		return false;
+
 	/*
 	 * We have tickets queued, bail so we don't compete with the async
 	 * flushers.
#4 lore raw
In reply to Josef Bacik
Quoted 23 lines · Josef Bacik

Hello,

I thought I had fixed the preemptive flushing burning CPU's problem with my previous set of fixes, but I was wrong. However those tracepoints gave me the information I needed to fix the problem properly. The first patch

btrfs: reduce the preemptive flushing threshold to 90%

can go back to stable and make its way into the distros to stop the pain for the current users having problems. The second patch augments the fix with a little less of a strong hammer.

The problem is for very full file systems on slower disks will end up with a very small threshold to start preemptive flushing. We were relying on sanity checks to bail out ahead of time, however they were not strong enough. These problematic cases existed in the short area where there was enough space to operate without needing to do synchronous flushing, but not enough space to avoid flushing all of the time.

The fix is to adjust the sanity checks to something more reasonable to account for these cases and avoid spinning doing preemptive flushing constantly. Thanks,

Quoted 5 lines

Josef

Josef Bacik (2):
btrfs: reduce the preemptive flushing threshold to 90%
btrfs: do not do preemptive flushing if the majority is global rsv

Added to misc-next, thanks.