| Message ID | cover.1605736355.git.boris@bur.io (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: free space tree mounting fixes | expand |
On Wed, Nov 18, 2020 at 03:06:15PM -0800, Boris Burkov wrote: > This patch set cleans up issues surrounding enabling and disabling various > free space cache features and their reporting in /proc/mounts. Because the > improvements became somewhat complex, the series starts by lifting rw mount > logic into a single place. > > The first patch is a setup patch that unifies very similar logic between a > normal rw mount and a ro->rw remount. This is a useful setup step for adding > more functionality to ro->rw remounts. > > The second patch fixes the omission of orphan inode cleanup on a few trees > during ro->rw remount. > > The third patch stops marking block groups with need_free_space when the > free space tree is not yet enabled. > > The fourth patch adds enabling the free space tree to ro->rw remount. > > The fifth patch adds a method for clearing oneshot mount options after mount. > > The sixth patch adds support for clearing the free space tree on ro->rw remount. > > The seventh patch sets up for more accurate /proc/mounts by ensuring that > cache_generation > 0 iff space_cache is enabled. > > The eigth patch is the more accurate /proc/mounts logic. > > The ninth patch is a convenience kernel message that complains when we skip > changing the free space tree on remount. > > The tenth patch removes the space cache v1 free space item and free space > inodes when space cache v1 is disabled (nospace_cache or space_cache=v2). > > The eleventh patch stops re-creating the free space objects when we are not > using space_cache=v1 > > The twelfth patch fixes a lockdep failure in creating the free space tree. Is this fixing a problem caused by some patches in this series? Because if yes, the fix should be folded there. A standalone patch makese sense in case we can't fold it there (eg. after merging to Linus' tree), otherwise the merged patchsets should be made of complete patches, without fixes-to-fixes. Even if the patchset is in misc-next, fixups are still doable.
On Fri, Nov 20, 2020 at 10:32:22PM +0100, David Sterba wrote: > On Wed, Nov 18, 2020 at 03:06:15PM -0800, Boris Burkov wrote: > > This patch set cleans up issues surrounding enabling and disabling various > > free space cache features and their reporting in /proc/mounts. Because the > > improvements became somewhat complex, the series starts by lifting rw mount > > logic into a single place. > > > > The first patch is a setup patch that unifies very similar logic between a > > normal rw mount and a ro->rw remount. This is a useful setup step for adding > > more functionality to ro->rw remounts. > > > > The second patch fixes the omission of orphan inode cleanup on a few trees > > during ro->rw remount. > > > > The third patch stops marking block groups with need_free_space when the > > free space tree is not yet enabled. > > > > The fourth patch adds enabling the free space tree to ro->rw remount. > > > > The fifth patch adds a method for clearing oneshot mount options after mount. > > > > The sixth patch adds support for clearing the free space tree on ro->rw remount. > > > > The seventh patch sets up for more accurate /proc/mounts by ensuring that > > cache_generation > 0 iff space_cache is enabled. > > > > The eigth patch is the more accurate /proc/mounts logic. > > > > The ninth patch is a convenience kernel message that complains when we skip > > changing the free space tree on remount. > > > > The tenth patch removes the space cache v1 free space item and free space > > inodes when space cache v1 is disabled (nospace_cache or space_cache=v2). > > > > The eleventh patch stops re-creating the free space objects when we are not > > using space_cache=v1 > > > > The twelfth patch fixes a lockdep failure in creating the free space tree. > > Is this fixing a problem caused by some patches in this series? Because > if yes, the fix should be folded there. A standalone patch makese sense > in case we can't fold it there (eg. after merging to Linus' tree), > otherwise the merged patchsets should be made of complete patches, > without fixes-to-fixes. Even if the patchset is in misc-next, fixups are > still doable. The new 'needs_free_space' patch (#3) fixes the bug in this series that you caught, so if I understand correctly, I should appropriately fold that one into one of the existing patches. The lockdep issue exists in misc-next as far as I can tell, so I think a standalone patch makes sense. Let me know if I misunderstood you on either of those. Thanks, Boris
On Mon, Nov 30, 2020 at 10:24:16AM -0800, Boris Burkov wrote: > On Fri, Nov 20, 2020 at 10:32:22PM +0100, David Sterba wrote: > > On Wed, Nov 18, 2020 at 03:06:15PM -0800, Boris Burkov wrote: > > Is this fixing a problem caused by some patches in this series? Because > > if yes, the fix should be folded there. A standalone patch makese sense > > in case we can't fold it there (eg. after merging to Linus' tree), > > otherwise the merged patchsets should be made of complete patches, > > without fixes-to-fixes. Even if the patchset is in misc-next, fixups are > > still doable. > > The new 'needs_free_space' patch (#3) fixes the bug in this series that > you caught, so if I understand correctly, I should appropriately fold > that one into one of the existing patches. I'll fold it but which one, 1 is only refactoring code and 2 starting orphan cleanup, so I think it's 2. I have run the test on each commit with the same test steps as be fore but can't reproduce it (in a VM, previously it was another box). > The lockdep issue exists in misc-next as far as I can tell, so I think a > standalone patch makes sense. Ok.
On Wed, Nov 18, 2020 at 03:06:15PM -0800, Boris Burkov wrote: > This patch set cleans up issues surrounding enabling and disabling various > free space cache features and their reporting in /proc/mounts. Because the > improvements became somewhat complex, the series starts by lifting rw mount > logic into a single place. > > Boris Burkov (12): > btrfs: lift rw mount setup from mount and remount > btrfs: cleanup all orphan inodes on ro->rw remount > btrfs: only mark bg->needs_free_space if free space tree is on > btrfs: create free space tree on ro->rw remount > btrfs: clear oneshot options on mount and remount > btrfs: clear free space tree on ro->rw remount > btrfs: keep sb cache_generation consistent with space_cache > btrfs: use sb state to print space_cache mount option > btrfs: warn when remount will not change the free space tree > btrfs: remove free space items when disabling space cache v1 > btrfs: skip space_cache v1 setup when not using it > btrfs: fix lockdep error creating free space tree Moved from topic branch to misc-next, with some more fixups. I can do some fixups until the end of this week. We need test coverage for various ro->rw and v1/v2/no transitions, I'm aware you sent something for fstests but haven't looked so far. The whole core for the v1->v2 conversion is there, from now on please send fixes as new patches. Thanks.
This patch set cleans up issues surrounding enabling and disabling various free space cache features and their reporting in /proc/mounts. Because the improvements became somewhat complex, the series starts by lifting rw mount logic into a single place. The first patch is a setup patch that unifies very similar logic between a normal rw mount and a ro->rw remount. This is a useful setup step for adding more functionality to ro->rw remounts. The second patch fixes the omission of orphan inode cleanup on a few trees during ro->rw remount. The third patch stops marking block groups with need_free_space when the free space tree is not yet enabled. The fourth patch adds enabling the free space tree to ro->rw remount. The fifth patch adds a method for clearing oneshot mount options after mount. The sixth patch adds support for clearing the free space tree on ro->rw remount. The seventh patch sets up for more accurate /proc/mounts by ensuring that cache_generation > 0 iff space_cache is enabled. The eigth patch is the more accurate /proc/mounts logic. The ninth patch is a convenience kernel message that complains when we skip changing the free space tree on remount. The tenth patch removes the space cache v1 free space item and free space inodes when space cache v1 is disabled (nospace_cache or space_cache=v2). The eleventh patch stops re-creating the free space objects when we are not using space_cache=v1 The twelfth patch fixes a lockdep failure in creating the free space tree. changes for v7: Rebase onto newer misc-next Patch 1/12: No change. Patch 2/12: No change. Patch 3/12: New. Patch 4/12: No change. Patch 5/12: Moved unused cache_opt variable to patch 7. Patch 6/12: No change. Patch 7/12: Declare cache_opt here rather than Patch 5. Actually clear clear_cache as in the patch description. Patch 8/12: No change. Patch 9/12: No change. Patch 10/12: No change. Patch 11/12: No change. Patch 12/12: New. changes for v6: Rebase all patches on newer misc-next Patch 1/10: Fix unnecessary re-ordering of discard resume and most of mount_rw. Patch 2/10: No change. Patch 3/10: No change. Patch 4/10: Handle unclean rebase. Patch 5/10: No change. Patch 6/10: No change. Patch 7/10: No change. Patch 8/10: No change. Patch 9/10: No change. Patch 10/10: No change. changes for v5: Patch 1/10: No change. Patch 2/10: No change. Patch 3/10: No change. Patch 4/10: New. Patch 5/10: New. Patch 6/10: Handles remounts for no<->v1. Was Patch 4. Patch 7/10: No change. Was Patch 5 Patch 8/10: Warns/undoes flags on skipped clear as well as skipped create. Was Patch 6. Patch 9/10: No change. Was Patch 7. Patch 10/10: No change. Was Patch 8. changes for v4: Patch 1/8: New Patch 2/8: New Patch 3/8: (was Patch 1) Made much simpler by lifting of Patch 1. Simply add free space tree creation to lifted rw mount logic. Patch 4/8: Split out from old Patch 2. Add an fs_info flag to avoid surprises when a different transaction updates the cache_generation. Patch 5/8: Split out from old Patch 2, but no change logically. Patch 6/8: Split out from old Patch 1. Formatting fixes. Patch 7/8: (was Patch 3) Rely on delayed_iput running after orphan cleanup (setup in patch 2) to pull iputs out of mount path, per review. Patch 8/8: No change. changes for v3: Patch 1/4: Change failure to warning logging. Patch 2/4: New; fixes mount option printing. Patch 3/4: Fix orphan inode vs. delayed iput bug, change remove function to take inode as a sink. Patch 4/4: No changes. changes for v2: Patch 1/3: made remount _only_ work in ro->rw case, added comment. Patch 2/3: added btrfs_ prefix to non-static function, removed bad whitespace. Boris Burkov (12): btrfs: lift rw mount setup from mount and remount btrfs: cleanup all orphan inodes on ro->rw remount btrfs: only mark bg->needs_free_space if free space tree is on btrfs: create free space tree on ro->rw remount btrfs: clear oneshot options on mount and remount btrfs: clear free space tree on ro->rw remount btrfs: keep sb cache_generation consistent with space_cache btrfs: use sb state to print space_cache mount option btrfs: warn when remount will not change the free space tree btrfs: remove free space items when disabling space cache v1 btrfs: skip space_cache v1 setup when not using it btrfs: fix lockdep error creating free space tree fs/btrfs/block-group.c | 45 ++------ fs/btrfs/ctree.h | 3 + fs/btrfs/disk-io.c | 205 +++++++++++++++++++++--------------- fs/btrfs/disk-io.h | 2 + fs/btrfs/free-space-cache.c | 121 +++++++++++++++++++++ fs/btrfs/free-space-cache.h | 6 ++ fs/btrfs/super.c | 70 ++++++------ fs/btrfs/transaction.c | 2 + 8 files changed, 294 insertions(+), 160 deletions(-)