On Thu, Oct 19, 2017 at 08:59:36AM +0200, Christoph Hellwig wrote:
> The define was always set to 1, which means looping until we reach is
> was dead code from the start.
>
> Also remove an initialization of next_fsb for the done case that doesn't
> fit the new code flow - it was never checked by the caller in the done
> case to start with.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 47 ++++++++++++++++++++---------------------------
> fs/xfs/libxfs/xfs_bmap.h | 12 +-----------
> fs/xfs/xfs_bmap_util.c | 14 ++------------
> 3 files changed, 23 insertions(+), 50 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 22e7578e5696..89ea8a5235c0 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5692,8 +5692,7 @@ xfs_bmse_shift_one(
> /*
> * Shift extent records to the left/right to cover/create a hole.
> *
> - * The maximum number of extents to be shifted in a single operation is
> - * @num_exts. @stop_fsb specifies the file offset at which to stop shift and the
> + * @stop_fsb specifies the file offset at which to stop shift and the
> * file offset where we've left off is returned in @next_fsb. @offset_shift_fsb
> * is the length by which each extent is shifted. If there is no hole to shift
> * the extents into, this will be considered invalid operation and we abort
> @@ -5709,14 +5708,12 @@ xfs_bmap_shift_extents(
> xfs_fileoff_t stop_fsb,
> xfs_fsblock_t *firstblock,
> struct xfs_defer_ops *dfops,
> - enum shift_direction direction,
> - int num_exts)
> + enum shift_direction direction)
> {
> struct xfs_btree_cur *cur = NULL;
> struct xfs_bmbt_irec got;
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp;
> - xfs_extnum_t nexts = 0;
> xfs_extnum_t current_ext;
> xfs_extnum_t total_extents;
> xfs_extnum_t stop_extent;
> @@ -5814,31 +5811,27 @@ xfs_bmap_shift_extents(
> }
> }
>
> - while (nexts++ < num_exts) {
> - error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> - ¤t_ext, &got, cur, &logflags,
> - direction, dfops);
> - if (error)
> - goto del_cursor;
> - /*
> - * If there was an extent merge during the shift, the extent
> - * count can change. Update the total and grade the next record.
> - */
> - if (direction == SHIFT_LEFT) {
> - total_extents = xfs_iext_count(ifp);
> - stop_extent = total_extents;
> - }
> + error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> + ¤t_ext, &got, cur, &logflags,
> + direction, dfops);
> + if (error)
> + goto del_cursor;
> + /*
> + * If there was an extent merge during the shift, the extent
> + * count can change. Update the total and grade the next record.
/me wonders what "grade the next record" means, but afaict this comment
disappears anyway...
--D
> + */
> + if (direction == SHIFT_LEFT) {
> + total_extents = xfs_iext_count(ifp);
> + stop_extent = total_extents;
> + }
>
> - if (current_ext == stop_extent) {
> - *done = 1;
> - *next_fsb = NULLFSBLOCK;
> - break;
> - }
> - xfs_iext_get_extent(ifp, current_ext, &got);
> + if (current_ext == stop_extent) {
> + *done = 1;
> + goto del_cursor;
> }
> + xfs_iext_get_extent(ifp, current_ext, &got);
>
> - if (!*done)
> - *next_fsb = got.br_startoff;
> + *next_fsb = got.br_startoff;
>
> del_cursor:
> if (cur)
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index c837e88ba19a..985b1c26566a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -183,15 +183,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> !isnullstartblock(irec->br_startblock);
> }
>
> -/*
> - * This macro is used to determine how many extents will be shifted
> - * in one write transaction. We could require two splits,
> - * an extent move on the first and an extent merge on the second,
> - * So it is proper that one extent is shifted inside write transaction
> - * at a time.
> - */
> -#define XFS_BMAP_MAX_SHIFT_EXTENTS 1
> -
> enum shift_direction {
> SHIFT_LEFT = 0,
> SHIFT_RIGHT,
> @@ -239,8 +230,7 @@ uint xfs_default_attroffset(struct xfs_inode *ip);
> int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
> int *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
> - struct xfs_defer_ops *dfops, enum shift_direction direction,
> - int num_exts);
> + struct xfs_defer_ops *dfops, enum shift_direction direction);
> int xfs_bmap_split_extent(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,
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 47b53c88de7c..3273f083c496 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1359,14 +1359,9 @@ xfs_collapse_file_space(
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>
> xfs_defer_init(&dfops, &first_block);
> -
> - /*
> - * We are using the write transaction in which max 2 bmbt
> - * updates are allowed
> - */
> error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> &done, stop_fsb, &first_block, &dfops,
> - SHIFT_LEFT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + SHIFT_LEFT);
> if (error)
> goto out_bmap_cancel;
>
> @@ -1438,14 +1433,9 @@ xfs_insert_file_space(
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> xfs_defer_init(&dfops, &first_block);
> -
> - /*
> - * We are using the write transaction in which max 2 bmbt
> - * updates are allowed
> - */
> error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> &done, stop_fsb, &first_block, &dfops,
> - SHIFT_RIGHT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + SHIFT_RIGHT);
> if (error)
> goto out_bmap_cancel;
>
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
