diff mbox series

btrfs: only enable extent map shrinker for DEBUG builds

Message ID 09ca70ddac244d13780bd82866b8b708088362fb.1723770634.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: only enable extent map shrinker for DEBUG builds | expand

Commit Message

Qu Wenruo Aug. 16, 2024, 1:10 a.m. UTC
Although there are several patches improving the extent map shrinker,
there are still reports of too frequent shrinker behavior, taking too
much CPU for the kswapd process.

So let's only enable extent shrinker for now, until we got more
comprehensive understanding and a better solution.

Link: https://lore.kernel.org/linux-btrfs/3df4acd616a07ef4d2dc6bad668701504b412ffc.camel@intelfx.name/
Link: https://lore.kernel.org/linux-btrfs/c30fd6b3-ca7a-4759-8a53-d42878bf84f7@gmail.com/
Fixes: 956a17d9d050 ("btrfs: add a shrinker for extent maps")
CC: stable@vger.kernel.org # 6.10+
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
I also checked how XFS (the only other fs implemented the
free_cached_objects callback) implemented the callback.

They did two things:

- Make sure there is only one queued reclaim
  Currently we only do the reclaim for kswapd, but for multi-node
  systems, we can still have multiple kswapd processes.

  But I do not think that's the root cause.

- With an extra delay of 60% of xfs_syncd_centiseccs
  The default value for xfs_syncd_centiseccs is 3000 centiseconds (30s),
  with a minimal 100 centiseconds (1s).
  This results the reclaim work only to be executed at most every 18
  seconds by default (or 0.6s for the minimal interval).

  I believe this is the root cause, we have no extra delay and that
  makes btrfs to shrink extent maps too frequently.
---
 fs/btrfs/super.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

David Sterba Aug. 16, 2024, 7:18 p.m. UTC | #1
On Fri, Aug 16, 2024 at 10:40:38AM +0930, Qu Wenruo wrote:
> Although there are several patches improving the extent map shrinker,
> there are still reports of too frequent shrinker behavior, taking too
> much CPU for the kswapd process.
> 
> So let's only enable extent shrinker for now, until we got more
> comprehensive understanding and a better solution.

Thanks, that's probably better than to disable it completely. I'll
forward to patch so we get it to sable next week.

Reviewed-by: David Sterba <dsterba@suse.com>
diff mbox series

Patch

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 11044e9e2cb1..98fa0f382480 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2402,7 +2402,13 @@  static long btrfs_nr_cached_objects(struct super_block *sb, struct shrink_contro
 
 	trace_btrfs_extent_map_shrinker_count(fs_info, nr);
 
-	return nr;
+	/*
+	 * Only report the real number for DEBUG builds, as there are reports of
+	 * serious performance degradation caused by too frequent shrinks.
+	 */
+	if (IS_ENABLED(CONFIG_BTRFS_DEBUG))
+		return nr;
+	return 0;
 }
 
 static long btrfs_free_cached_objects(struct super_block *sb, struct shrink_control *sc)