On Thu, Mar 18, 2021 at 03:33:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Fix the weird split of responsibilities between xfs_can_free_eofblocks
> and xfs_free_eofblocks by moving the chunk of code that looks for any
> actual post-EOF space mappings from the second function into the first.
>
> This clears the way for deferred inode inactivation to be able to decide
> if an inode needs inactivation work before committing the released inode
> to the inactivation code paths (vs. marking it for reclaim).
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_bmap_util.c | 148 +++++++++++++++++++++++++-----------------------
> 1 file changed, 78 insertions(+), 70 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index e7d68318e6a5..d4ceba5370c7 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -597,8 +597,17 @@ xfs_bmap_punch_delalloc_range(
> * regular files that are marked preallocated or append-only.
> */
> bool
> -xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
> +xfs_can_free_eofblocks(
> + struct xfs_inode *ip,
> + bool force)
> {
> + struct xfs_bmbt_irec imap;
> + struct xfs_mount *mp = ip->i_mount;
> + xfs_fileoff_t end_fsb;
> + xfs_fileoff_t last_fsb;
> + int nimaps = 1;
> + int error;
Should we have an assert here that this is called under the iolock?
Or can't the reclaim be expressed nicely?
> +/*
> + * This is called to free any blocks beyond eof. The caller must hold
> + * IOLOCK_EXCL unless we are in the inode reclaim path and have the only
> + * reference to the inode.
> + */
Same thing here, usually asserts are better than comments.. That being
said can_free_eofblocks would benefit from at least a comment if the
assert doesn't work.
Otherwise this looks good.
