mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Tal Zussman <tz2294@columbia.edu>
Cc: David Sterba <dsterba@suse.com>, Chris Mason <mason@kernel.org>,
	Qu Wenruo <wqu@suse.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 00/18] btrfs: remove the v1 space cache
Date: Tue, 13 Oct 2026 14:22:14 +0200	[thread overview]
Message-ID: <20261013122214.GA1310987@suse.cz> (raw)
In-Reply-To: <20261012-btrfs-remove-v1-space-cache-v4-0-f125bd8c07f9@columbia.edu>

On Mon, Oct 12, 2026 at 12:28:12PM -0400, Tal Zussman wrote:
> Since commit 545e560a5b0f ("btrfs: disable v1 space cache") the mount
> options can't select the v1 space cache anymore, but the code is all
> still there, and a filesystem with an old cache and no free space tree
> still enabled it from the superblock. Qu suggested removing it rather
> than converting its page handling to folios [1].
> 
> Patch 1 fixes a pre-existing leak in the cleanup path, where a crash
> before the delayed iput evicts a cache inode left it unreachable with
> its link count still set.
> 
> Patch 2 stops enabling the cache from the on-disk state, so an existing
> cache is cleaned up on the next read-write mount, as -o nospace_cache
> already did. This is the one user-visible change: the cleanup is now
> unconditional, and a read-write mount fails if it fails. There are two
> ways to fix this failure:
> 
> 1. Convert to the free space tree during mount (the recommended
>    conversion), but this could fail for the same reason.
> 
> 2. Run 'btrfs rescue clear-space-cache v1' on an unmounted filesystem.
> 
> Patches 3-7 remove the write path, 8 and 9 the load path and
> disk_cache_state, and 10 and 11 the SPACE_CACHE flag and the unused
> half of the cleanup helper. Patches 12-17 remove the trimming ranges
> and the free space inode special cases in the write path, which only
> the v1 writer used. Patch 18 removes TRANS_JOIN_NOLOCK, which the space
> cache was the only user of.
> 
> What's left is what's needed to find and delete the cache inodes of an
> existing filesystem:
> 
> 1. lookup_free_space_inode(), btrfs_remove_free_space_inode(), and
>    btrfs_cleanup_free_space_cache_v1(), which runs on the first
>    read-write mount and zeroes cache_generation in the super block.
> 
> 2. btrfs_truncate_free_space_cache() and delete_v1_space_cache(), which
>    relocation uses to get a cache inode's extents out of a block group.
> 
> 3. btrfs_is_free_space_inode(), for the evict and inode update paths.
> 
> 4. The on-disk definitions: cache_generation in the super block,
>    BTRFS_FREE_SPACE_OBJECTID, and the free space header and entry
>    items.
> 
> space_cache and space_cache=v1 still fall back to nospace_cache with a
> warning.
> 
> This also removes the page-based I/O in the free space code.
> btrfs_io_ctl used a struct page array and was one of the last users
> of clear_page_dirty_for_io().
> 
> Tested with fstests (btrfs and generic quick groups), with and without
> the free space tree, with no regressions against the base kernel.
> Upgrading from a filesystem with an existing v1 cache written by a 6.8
> kernel was also tested. The cache is removed on the first read-write
> mount and on remount from read-only, and the resulting filesystem is
> clean under btrfs check and still mountable by the old kernel.
> 
> Based on the misc-next commit in btrfs for-next (a5c77fde6730).
> 
> [1] https://lore.kernel.org/linux-btrfs/e1dd5a75-9a67-4bc6-b8d8-bfff79b5b907@suse.com/
> 
> ---
> Changes in v4:
> - Patch 1 (new): Write the zero link count when removing a free space
>   inode, so an interrupted cleanup can't leak it.
> - Patch 2: Clear FREE_SPACE_TREE in btrfs_reconfigure() whenever the
>   free space tree is not on disk, so a remount that only warns about
>   the free space tree no longer leaves the option set, per Sashiko.
> - Patch 2: Dropped review tag.
> - Patch 16: Also drop the tree root check from the delalloc metadata
>   release.
> - Patch 18: Redo against the TRANS_* flag cleanups in for-next.
> - Link to v3: https://patch.msgid.link/20260916-btrfs-remove-v1-space-cache-v3-0-10ebde03b96f@columbia.edu

Thanks, unfortunatelly this did not make it into the 7.4 pull as the
freeze was yesterday and I was not able to process the v3 last week.
The v4 still has some issues reported by sashiko. It could be still
possible to send that separately as 2nd pull to 7.4 because the code has
been in linux-next, small fixups would be possible, and it's code
removal.

  parent reply	other threads:[~2026-10-13 12:22 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-12 16:28 Tal Zussman
2026-10-12 16:28 ` [PATCH v4 01/18] btrfs: update the inode item when removing a free space inode Tal Zussman
2026-10-12 16:28 ` [PATCH v4 02/18] btrfs: stop enabling the v1 space cache from the on-disk state Tal Zussman
2026-10-12 16:28 ` [PATCH v4 03/18] btrfs: remove the v1 space cache writeout from the transaction commit Tal Zussman
2026-10-12 16:28 ` [PATCH v4 04/18] btrfs: remove the free space cache endio workqueue Tal Zussman
2026-10-12 16:28 ` [PATCH v4 05/18] btrfs: remove the v1 space cache write path Tal Zussman
2026-10-12 16:28 ` [PATCH v4 06/18] btrfs: rename cache_write_mutex to dirty_bgs_update_mutex Tal Zussman
2026-10-12 16:28 ` [PATCH v4 07/18] btrfs: drop the transaction handle from the prealloc helpers Tal Zussman
2026-10-12 16:28 ` [PATCH v4 08/18] btrfs: remove the v1 space cache load path Tal Zussman
2026-10-12 16:28 ` [PATCH v4 09/18] btrfs: remove btrfs_disk_cache_state Tal Zussman
2026-10-12 16:28 ` [PATCH v4 10/18] btrfs: remove the SPACE_CACHE mount option flag Tal Zussman
2026-10-12 16:28 ` [PATCH v4 11/18] btrfs: replace btrfs_set_free_space_cache_v1_active() with a cleanup helper Tal Zussman
2026-10-12 16:28 ` [PATCH v4 12/18] btrfs: remove the free space cache trimming ranges Tal Zussman
2026-10-12 16:28 ` [PATCH v4 13/18] btrfs: remove BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE Tal Zussman
2026-10-12 16:28 ` [PATCH v4 14/18] btrfs: remove the free space inode ordered extent special cases Tal Zussman
2026-10-12 16:28 ` [PATCH v4 15/18] btrfs: remove the free space inode special cases from the COW paths Tal Zussman
2026-10-12 16:28 ` [PATCH v4 16/18] btrfs: stop special-casing free space inodes in the delalloc accounting Tal Zussman
2026-10-12 16:28 ` [PATCH v4 17/18] btrfs: stop reading free space inodes from the commit root Tal Zussman
2026-10-12 16:28 ` [PATCH v4 18/18] btrfs: remove TRANS_JOIN_NOLOCK Tal Zussman
2026-10-13 12:22 ` David Sterba [this message]
2026-10-14 18:00   ` [PATCH v4 00/18] btrfs: remove the v1 space cache Tal Zussman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261013122214.GA1310987@suse.cz \
    --to=dsterba@suse.cz \
    --cc=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mason@kernel.org \
    --cc=tz2294@columbia.edu \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®