On Tue, Dec 17, 2019 at 10:02:42AM -0700, Allison Collins wrote:
> On 12/13/19 10:12 AM, Brian Foster wrote:
> > The insert range operation currently splits the extent at the target
> > offset in a separate transaction and lock cycle from the one that
> > shifts extents. In preparation for reworking insert range into an
> > atomic operation, lift the code into the caller so it can be easily
> > condensed to a single rolling transaction and lock cycle and
> > eliminate the helper. No functional changes.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> Looks ok to me.
>
> Reviewed by: Allison Collins <allison.henderson@xxxxxxxxxx>
"Reviewed-by", with dash?
--D
> > ---
> > fs/xfs/libxfs/xfs_bmap.c | 32 ++------------------------------
> > fs/xfs/libxfs/xfs_bmap.h | 3 ++-
> > fs/xfs/xfs_bmap_util.c | 14 +++++++++++++-
> > 3 files changed, 17 insertions(+), 32 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index a9ad1f991ba3..2bba0f983e4f 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -6021,8 +6021,8 @@ xfs_bmap_insert_extents(
> > * @split_fsb is a block where the extents is split. If split_fsb lies in a
> > * hole or the first block of extents, just return 0.
> > */
> > -STATIC int
> > -xfs_bmap_split_extent_at(
> > +int
> > +xfs_bmap_split_extent(
> > struct xfs_trans *tp,
> > struct xfs_inode *ip,
> > xfs_fileoff_t split_fsb)
> > @@ -6138,34 +6138,6 @@ xfs_bmap_split_extent_at(
> > return error;
> > }
> > -int
> > -xfs_bmap_split_extent(
> > - struct xfs_inode *ip,
> > - xfs_fileoff_t split_fsb)
> > -{
> > - struct xfs_mount *mp = ip->i_mount;
> > - struct xfs_trans *tp;
> > - int error;
> > -
> > - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write,
> > - XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
> > - if (error)
> > - return error;
> > -
> > - xfs_ilock(ip, XFS_ILOCK_EXCL);
> > - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> > -
> > - error = xfs_bmap_split_extent_at(tp, ip, split_fsb);
> > - if (error)
> > - goto out;
> > -
> > - return xfs_trans_commit(tp);
> > -
> > -out:
> > - xfs_trans_cancel(tp);
> > - return error;
> > -}
> > -
> > /* Deferred mapping is only for real extents in the data fork. */
> > static bool
> > xfs_bmap_is_update_needed(
> > diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> > index 14d25e0b7d9c..f3259ad5c22c 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.h
> > +++ b/fs/xfs/libxfs/xfs_bmap.h
> > @@ -222,7 +222,8 @@ int xfs_bmap_can_insert_extents(struct xfs_inode *ip, xfs_fileoff_t off,
> > int xfs_bmap_insert_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> > xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
> > bool *done, xfs_fileoff_t stop_fsb);
> > -int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
> > +int xfs_bmap_split_extent(struct xfs_trans *tp, struct xfs_inode *ip,
> > + xfs_fileoff_t split_offset);
> > int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
> > xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
> > struct xfs_bmbt_irec *got, struct xfs_iext_cursor *cur,
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 2efd78a9719e..829ab1a804c9 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -1139,7 +1139,19 @@ xfs_insert_file_space(
> > * is not the starting block of extent, we need to split the extent at
> > * stop_fsb.
> > */
> > - error = xfs_bmap_split_extent(ip, stop_fsb);
> > + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write,
> > + XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
> > + if (error)
> > + return error;
> > +
> > + xfs_ilock(ip, XFS_ILOCK_EXCL);
> > + xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> > +
> > + error = xfs_bmap_split_extent(tp, ip, stop_fsb);
> > + if (error)
> > + goto out_trans_cancel;
> > +
> > + error = xfs_trans_commit(tp);
> > if (error)
> > return error;
> >
