mbox series

[00/18] Truncate cleanups and preparation work

Message ID cover.1638569556.git.josef@toxicpanda.com (mailing list archive)
Headers show
Series Truncate cleanups and preparation work | expand

Message

Josef Bacik Dec. 3, 2021, 10:18 p.m. UTC
Hello,

The first thing I'm implementing with the garbage collection tree is
btrfs_truncate_inode_items() on evicted inodes.  However
btrfs_truncate_inode_items() has a lot of oddities that it's grown over the
years, and requires having a valid btrfs_inode to use.  We don't really want to
have to look up the old inode to do the truncate, we just want to do the tree
operaitons to delete all of the objects and extents.

Enter this patch series, I've cleaned up btrfs_truncate_inode_items(), moved as
much of the inode operations out to the respective callers, and cleaned up the
argument passing and such to make it a little cleaner.

We still have to pass in the inode for the ^NO_HOLES case for the normal
truncate path, but other than that I've stripped it down so that we can pass in
a NULL inode and get all the work done.

This has the nice side-effect of cleaning up a lot of our

if (root == LOG_ROOT)
	// do something else

checks in this helper, and hopefully makes it more straightforward to
understand.  Thanks,

Josef

Josef Bacik (18):
  btrfs: add an inode-item.h
  btrfs: move btrfs_truncate_inode_items to inode-item.c
  btrfs: move extent locking outside of btrfs_truncate_inode_items
  btrfs: remove free space cache inode check in
    btrfs_truncate_inode_items
  btrfs: move btrfs_kill_delayed_inode_items into evict
  btrfs: remove found_extent from btrfs_truncate_inode_items
  btrfs: add btrfs_truncate_control struct
  btrfs: only update i_size in truncate paths that care
  btrfs: only call inode_sub_bytes in truncate paths that care
  btrfs: control extent reference updates with a control flag for
    truncate
  btrfs: use a flag to control when to clear the file extent range
  btrfs: pass the ino via btrfs_truncate_control
  btrfs: add inode to btrfs_truncate_control
  btrfs: convert BUG_ON() in btrfs_truncate_inode_items to ASSERT
  btrfs: convert BUG() for pending_del_nr into an ASSERT
  btrfs: combine extra if statements in btrfs_truncate_inode_items
  btrfs: make should_throttle loop local in btrfs_truncate_inode_items
  btrfs: do not check -EAGAIN when truncating inodes in the log root

 fs/btrfs/ctree.h            |  34 ---
 fs/btrfs/delayed-inode.c    |   1 +
 fs/btrfs/free-space-cache.c |  31 ++-
 fs/btrfs/inode-item.c       | 334 ++++++++++++++++++++++++++
 fs/btrfs/inode-item.h       |  86 +++++++
 fs/btrfs/inode.c            | 452 +++++-------------------------------
 fs/btrfs/relocation.c       |   1 +
 fs/btrfs/tree-log.c         |  15 +-
 8 files changed, 511 insertions(+), 443 deletions(-)
 create mode 100644 fs/btrfs/inode-item.h

Comments

Filipe Manana Dec. 6, 2021, 2:43 p.m. UTC | #1
On Fri, Dec 03, 2021 at 05:18:02PM -0500, Josef Bacik wrote:
> Hello,
> 
> The first thing I'm implementing with the garbage collection tree is
> btrfs_truncate_inode_items() on evicted inodes.  However
> btrfs_truncate_inode_items() has a lot of oddities that it's grown over the
> years, and requires having a valid btrfs_inode to use.  We don't really want to
> have to look up the old inode to do the truncate, we just want to do the tree
> operaitons to delete all of the objects and extents.
> 
> Enter this patch series, I've cleaned up btrfs_truncate_inode_items(), moved as
> much of the inode operations out to the respective callers, and cleaned up the
> argument passing and such to make it a little cleaner.
> 
> We still have to pass in the inode for the ^NO_HOLES case for the normal
> truncate path, but other than that I've stripped it down so that we can pass in
> a NULL inode and get all the work done.
> 
> This has the nice side-effect of cleaning up a lot of our
> 
> if (root == LOG_ROOT)
> 	// do something else
> 
> checks in this helper, and hopefully makes it more straightforward to
> understand.  Thanks,
> 
> Josef
> 
> Josef Bacik (18):
>   btrfs: add an inode-item.h
>   btrfs: move btrfs_truncate_inode_items to inode-item.c
>   btrfs: move extent locking outside of btrfs_truncate_inode_items
>   btrfs: remove free space cache inode check in
>     btrfs_truncate_inode_items
>   btrfs: move btrfs_kill_delayed_inode_items into evict
>   btrfs: remove found_extent from btrfs_truncate_inode_items
>   btrfs: add btrfs_truncate_control struct
>   btrfs: only update i_size in truncate paths that care
>   btrfs: only call inode_sub_bytes in truncate paths that care
>   btrfs: control extent reference updates with a control flag for
>     truncate
>   btrfs: use a flag to control when to clear the file extent range
>   btrfs: pass the ino via btrfs_truncate_control
>   btrfs: add inode to btrfs_truncate_control
>   btrfs: convert BUG_ON() in btrfs_truncate_inode_items to ASSERT
>   btrfs: convert BUG() for pending_del_nr into an ASSERT
>   btrfs: combine extra if statements in btrfs_truncate_inode_items
>   btrfs: make should_throttle loop local in btrfs_truncate_inode_items
>   btrfs: do not check -EAGAIN when truncating inodes in the log root
> 
>  fs/btrfs/ctree.h            |  34 ---
>  fs/btrfs/delayed-inode.c    |   1 +
>  fs/btrfs/free-space-cache.c |  31 ++-
>  fs/btrfs/inode-item.c       | 334 ++++++++++++++++++++++++++
>  fs/btrfs/inode-item.h       |  86 +++++++
>  fs/btrfs/inode.c            | 452 +++++-------------------------------
>  fs/btrfs/relocation.c       |   1 +
>  fs/btrfs/tree-log.c         |  15 +-
>  8 files changed, 511 insertions(+), 443 deletions(-)
>  create mode 100644 fs/btrfs/inode-item.h

Looks good, and it passed one iteration of fstests here.

For the whole series:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> 
> -- 
> 2.26.3
>
David Sterba Dec. 6, 2021, 9:06 p.m. UTC | #2
On Fri, Dec 03, 2021 at 05:18:02PM -0500, Josef Bacik wrote:
> Hello,
> 
> The first thing I'm implementing with the garbage collection tree is
> btrfs_truncate_inode_items() on evicted inodes.  However
> btrfs_truncate_inode_items() has a lot of oddities that it's grown over the
> years, and requires having a valid btrfs_inode to use.  We don't really want to
> have to look up the old inode to do the truncate, we just want to do the tree
> operaitons to delete all of the objects and extents.
> 
> Enter this patch series, I've cleaned up btrfs_truncate_inode_items(), moved as
> much of the inode operations out to the respective callers, and cleaned up the
> argument passing and such to make it a little cleaner.
> 
> We still have to pass in the inode for the ^NO_HOLES case for the normal
> truncate path, but other than that I've stripped it down so that we can pass in
> a NULL inode and get all the work done.
> 
> This has the nice side-effect of cleaning up a lot of our
> 
> if (root == LOG_ROOT)
> 	// do something else
> 
> checks in this helper, and hopefully makes it more straightforward to
> understand.  Thanks,
> 
> Josef
> 
> Josef Bacik (18):
>   btrfs: add an inode-item.h
>   btrfs: move btrfs_truncate_inode_items to inode-item.c
>   btrfs: move extent locking outside of btrfs_truncate_inode_items
>   btrfs: remove free space cache inode check in
>     btrfs_truncate_inode_items
>   btrfs: move btrfs_kill_delayed_inode_items into evict
>   btrfs: remove found_extent from btrfs_truncate_inode_items
>   btrfs: add btrfs_truncate_control struct
>   btrfs: only update i_size in truncate paths that care
>   btrfs: only call inode_sub_bytes in truncate paths that care
>   btrfs: control extent reference updates with a control flag for
>     truncate
>   btrfs: use a flag to control when to clear the file extent range
>   btrfs: pass the ino via btrfs_truncate_control
>   btrfs: add inode to btrfs_truncate_control
>   btrfs: convert BUG_ON() in btrfs_truncate_inode_items to ASSERT
>   btrfs: convert BUG() for pending_del_nr into an ASSERT
>   btrfs: combine extra if statements in btrfs_truncate_inode_items
>   btrfs: make should_throttle loop local in btrfs_truncate_inode_items
>   btrfs: do not check -EAGAIN when truncating inodes in the log root

Added to misc-next, thanks.