xfs: get rid of _ITER_{ABORT,CONTINUE}
v2 cover xfs: get rid of _ITER_{ABORT,CONTINUE}
Hi all,
I have recently realized that the _ITER_CONTINUE/_ITER_ABORT defines are a giant thinko -- the _CONTINUE variants map to zero, and -ECANCELED can handle the _ABORT case just fine. This series scrapes both of them out of the kernel.
In this v2 of the patch we add comments to point out the special meaning of 0 and -ECANCELED, which replace the _ITER_* defines.
--D
PATCH v2 1/2 xfs: remove all *_ITER_ABORT values
From: Darrick J. Wong <[email protected]>
Use -ECANCELED to signal "stop iterating" instead of these magical *_ITER_ABORT values, since it's duplicative.
--- fs/xfs/libxfs/xfs_btree.c | 12 +++++------- fs/xfs/libxfs/xfs_btree.h | 9 +++++++-- fs/xfs/libxfs/xfs_rmap.c | 10 +++++----- fs/xfs/libxfs/xfs_shared.h | 3 --- fs/xfs/scrub/agheader.c | 4 ++-- fs/xfs/scrub/attr.c | 2 +- fs/xfs/scrub/bmap.c | 4 ++-- fs/xfs/scrub/repair.c | 4 ++-- fs/xfs/xfs_dquot.c | 2 +- fs/xfs/xfs_fsmap.c | 4 ++-- fs/xfs/xfs_ioctl.c | 12 ++++++------ fs/xfs/xfs_itable.c | 6 +++--- fs/xfs/xfs_itable.h | 13 +++++++++---- fs/xfs/xfs_iwalk.c | 2 +- fs/xfs/xfs_iwalk.h | 11 ++++++++--- 15 files changed, 54 insertions(+), 44 deletions(-)
fs/xfs/libxfs/xfs_btree.c +5 -7
@@ -4598,7 +4598,7 @@ xfs_btree_simple_query_range(/* Callback */ error =fn (cur, recp, priv); -if (error <0 || error ==XFS_BTREE_QUERY_RANGE_ABORT ) +if (error)break ; advloop:
@@ -4700,8 +4700,7 @@ xfs_btree_overlapped_query_range(*/ if (ldiff >=0 && hdiff >=0 ) { error =fn (cur, recp, priv); -if (error <0 || - error ==XFS_BTREE_QUERY_RANGE_ABORT ) +if (error)break ; }else if (hdiff <0 ) {/* Record is larger than high key; pop. */
@@ -4772,8 +4771,7 @@ xfs_btree_overlapped_query_range(* Query a btree for all records overlapping a given interval of keys. The * supplied function will be called with each record found; return one of the * XFS_BTREE_QUERY_RANGE_{CONTINUE,ABORT} values or the usual negative error -* code. This function returns XFS_BTREE_QUERY_RANGE_ABORT, zero, or a -* negative error code. +* code. This function returns -ECANCELED, zero, or a negative error code. */ int xfs_btree_query_range(
@@ -4889,7 +4887,7 @@ xfs_btree_has_record_helper(union xfs_btree_rec *rec, void *priv) { - return XFS_BTREE_QUERY_RANGE_ABORT; + return -ECANCELED; }/* Is there a record covering a given range of keys? */
@@ -4904,7 +4902,7 @@ xfs_btree_has_record( error =xfs_btree_query_range (cur, low, high, &xfs_btree_has_record_helper,NULL ); -if (error ==XFS_BTREE_QUERY_RANGE_ABORT ) { +if (error == -ECANCELED ) { *exists = true;return 0 ; }
fs/xfs/libxfs/xfs_btree.h +7 -2
@@ -464,9 +464,14 @@ xfs_failaddr_t xfs_btree_lblock_verify(struct xfs_buf *bp,uint xfs_btree_compute_maxlevels (uint *limits,unsigned long len);unsigned long long xfs_btree_calc_size (uint *limits,unsigned long long len); -/* return codes */ +/* +* Return codes for the query range iterator function are 0 to continue +* iterating, and non-zero to stop iterating. Any non-zero value will be +* passed up to the _query_range caller. The special value -ECANCELED can be +* used to stop iteration, because _query_range never generates that error +* code on its own. +*/ #define XFS_BTREE_QUERY_RANGE_CONTINUE (XFS_ITER_CONTINUE)/* keep iterating */ -#define XFS_BTREE_QUERY_RANGE_ABORT (XFS_ITER_ABORT)/* stop iterating */ typedef int (*xfs_btree_query_range_fn )(struct xfs_btree_cur *cur,union xfs_btree_rec *rec,void *priv);
fs/xfs/libxfs/xfs_rmap.c +5 -5
@@ -261,7 +261,7 @@ xfs_rmap_find_left_neighbor_helper( *info->irec = *rec; *info->stat =1 ; -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ; }/*
@@ -304,7 +304,7 @@ xfs_rmap_find_left_neighbor( error =xfs_rmap_query_range (cur, &info.high, &info.high, xfs_rmap_find_left_neighbor_helper, &info); -if (error ==XFS_BTREE_QUERY_RANGE_ABORT ) +if (error == -ECANCELED ) error =0 ;if (*stat) trace_xfs_rmap_find_left_neighbor_result(cur->bc_mp,
@@ -338,7 +338,7 @@ xfs_rmap_lookup_le_range_helper( *info->irec = *rec; *info->stat =1 ; -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ; }/*
@@ -376,7 +376,7 @@ xfs_rmap_lookup_le_range( cur->bc_private.a.agno, bno, 0, owner, offset, flags); error =xfs_rmap_query_range (cur, &info.high, &info.high, xfs_rmap_lookup_le_range_helper, &info); -if (error ==XFS_BTREE_QUERY_RANGE_ABORT ) +if (error == -ECANCELED ) error =0 ;if (*stat) trace_xfs_rmap_lookup_le_range_result(cur->bc_mp,
@@ -2509,7 +2509,7 @@ xfs_rmap_has_other_keys_helper( ((rks->flags & rec->rm_flags) & XFS_RMAP_KEY_FLAGS) == rks->flags)return 0 ; rks->has_rmap = true; -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ; }/*
fs/xfs/libxfs/xfs_shared.h +0 -3
@@ -180,7 +180,4 @@ struct xfs_ino_geometry {/* Keep iterating the data structure. */ #define XFS_ITER_CONTINUE (0) -/* Stop iterating the data structure. */ -#define XFS_ITER_ABORT (1) -#endif /* __XFS_SHARED_H__ */
fs/xfs/scrub/agheader.c +2 -2
@@ -639,7 +639,7 @@ xchk_agfl_block(xchk_agfl_block_xref (sc, agbno);if (sc->sm->sm_flags &XFS_SCRUB_OFLAG_CORRUPT ) -return XFS_ITER_ABORT ; +return -ECANCELED ;return 0 ; }
@@ -730,7 +730,7 @@ xchk_agfl(/* Check the blocks in the AGFL. */ error =xfs_agfl_walk (sc->mp,XFS_BUF_TO_AGF (sc->sa.agf_bp), sc->sa.agfl_bp, xchk_agfl_block, &sai); -if (error ==XFS_ITER_ABORT ) { +if (error == -ECANCELED ) { error =0 ; goto out_free; }
fs/xfs/scrub/attr.c +1 -1
@@ -171,7 +171,7 @@ xchk_xattr_listent( args.blkno); fail_xref:if (sx->sc->sm->sm_flags &XFS_SCRUB_OFLAG_CORRUPT ) - context->seen_enough =XFS_ITER_ABORT ; + context->seen_enough = -ECANCELED ;return ; }
fs/xfs/scrub/bmap.c +2 -2
@@ -522,7 +522,7 @@ xchk_bmap_check_rmap( out:if (sc->sm->sm_flags &XFS_SCRUB_OFLAG_CORRUPT ) -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ;return 0 ; }
@@ -551,7 +551,7 @@ xchk_bmap_check_ag_rmaps( sbcri.sc = sc; sbcri.whichfork = whichfork; error =xfs_rmap_query_all (cur, xchk_bmap_check_rmap, &sbcri); -if (error ==XFS_BTREE_QUERY_RANGE_ABORT ) +if (error == -ECANCELED ) error =0 ;xfs_btree_del_cursor (cur, error);
fs/xfs/scrub/repair.c +2 -2
@@ -664,7 +664,7 @@ xrep_findroot_agfl_walk( {xfs_agblock_t *agbno = priv; -return (*agbno == bno) ?XFS_ITER_ABORT :0 ; +return (*agbno == bno) ? -ECANCELED :0 ; }/* Does this block match the btree information passed in? */
@@ -694,7 +694,7 @@ xrep_findroot_block(if (owner ==XFS_RMAP_OWN_AG ) { error =xfs_agfl_walk (mp, ri->agf, ri->agfl_bp, xrep_findroot_agfl_walk, &agbno); -if (error ==XFS_ITER_ABORT ) +if (error == -ECANCELED )return 0 ;if (error)return error;
fs/xfs/xfs_dquot.c +1 -1
@@ -1239,7 +1239,7 @@ xfs_qm_exit(void)/* * Iterate every dquot of a particular type. The caller must ensure that the * particular quota type is active. iter_fn can return negative error codes, -* or XFS_ITER_ABORT to indicate that it wants to stop iterating. +* or -ECANCELED to indicate that it wants to stop iterating. */ int xfs_qm_dqiterate(
fs/xfs/xfs_fsmap.c +2 -2
@@ -276,7 +276,7 @@ xfs_getfsmap_helper(*/ if (rec_daddr > info->next_daddr) {if (info->head->fmh_entries >= info->head->fmh_count) -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ; fmr.fmr_device = info->dev; fmr.fmr_physical = info->next_daddr;
@@ -295,7 +295,7 @@ xfs_getfsmap_helper(/* Fill out the extent we found */ if (info->head->fmh_entries >= info->head->fmh_count) -return XFS_BTREE_QUERY_RANGE_ABORT ; +return -ECANCELED ;trace_xfs_fsmap_mapping (mp, info->dev, info->agno, rec);
fs/xfs/xfs_ioctl.c +6 -6
@@ -831,7 +831,7 @@ xfs_bulkstat_fmt(/* * Check the incoming bulk request @hdr from userspace and initialize the * internal @breq bulk request appropriately. Returns 0 if the bulk request -* should proceed; XFS_ITER_ABORT if there's nothing to do; or the usual +* should proceed; -ECANCELED if there's nothing to do; or the usual * negative error code. */ static int
@@ -889,13 +889,13 @@ xfs_bulk_ireq_setup(/* Asking for an inode past the end of the AG? We're done! */ if (XFS_INO_TO_AGNO (mp, breq->startino) > hdr->agno) -return XFS_ITER_ABORT ; +return -ECANCELED ; }else if (hdr->agno)return -EINVAL ;/* Asking for an inode past the end of the FS? We're done! */ if (XFS_INO_TO_AGNO (mp, breq->startino) >= mp->m_sb.sb_agcount) -return XFS_ITER_ABORT ; +return -ECANCELED ;return 0 ; }
@@ -936,7 +936,7 @@ xfs_ioc_bulkstat(return -EFAULT ; error =xfs_bulk_ireq_setup (mp, &hdr, &breq, arg->bulkstat); -if (error ==XFS_ITER_ABORT ) +if (error == -ECANCELED ) goto out_teardown;if (error <0 )return error;
@@ -986,7 +986,7 @@ xfs_ioc_inumbers(return -EFAULT ; error =xfs_bulk_ireq_setup (mp, &hdr, &breq, arg->inumbers); -if (error ==XFS_ITER_ABORT ) +if (error == -ECANCELED ) goto out_teardown;if (error <0 )return error;
@@ -1881,7 +1881,7 @@ xfs_ioc_getfsmap( info.mp = ip->i_mount; info.data = arg; error =xfs_getfsmap (ip->i_mount, &xhead, xfs_getfsmap_format, &info); -if (error ==XFS_BTREE_QUERY_RANGE_ABORT ) { +if (error == -ECANCELED ) { error =0 ; aborted = true; }else if (error)
fs/xfs/xfs_itable.c +3 -3
@@ -137,7 +137,7 @@ xfs_bulkstat_one_int(xfs_irele (ip); error = bc->formatter (bc->breq, buf); -if (error ==XFS_IBULK_ABORT ) +if (error == -ECANCELED ) goto out_advance;if (error) goto out;
@@ -181,7 +181,7 @@ xfs_bulkstat_one(* If we reported one inode to userspace then we abort because we hit * the end of the buffer. Don't leak that back to userspace. */ -if (error ==XFS_IWALK_ABORT ) +if (error == -ECANCELED ) error =0 ;return error;
@@ -342,7 +342,7 @@ xfs_inumbers_walk(int error; error = ic->formatter (ic->breq, &inogrp); -if (error && error !=XFS_IBULK_ABORT ) +if (error && error != -ECANCELED )return error; ic->breq->startino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino) +
fs/xfs/xfs_itable.h +9 -4
@@ -18,9 +18,6 @@ struct xfs_ibulk {/* Only iterate within the same AG as startino */ #define XFS_IBULK_SAME_AG (XFS_IWALK_SAME_AG) -/* Return value that means we want to abort the walk. */ -#define XFS_IBULK_ABORT (XFS_IWALK_ABORT) -/* * Advance the user buffer pointer by one record of the given size. If the * buffer is now full, return the appropriate error code.
@@ -34,13 +31,21 @@ xfs_ibulk_advance( breq->ubuffer = b + bytes; breq->ocount++; -return breq->ocount == breq->icount ?XFS_IBULK_ABORT :0 ; +return breq->ocount == breq->icount ? -ECANCELED :0 ; }/* * Return stat information in bulk (by-inode) for the filesystem. */ +/* +* Return codes for the formatter function are 0 to continue iterating, and +* non-zero to stop iterating. Any non-zero value will be passed up to the +* bulkstat/inumbers caller. The special value -ECANCELED can be used to stop +* iteration, as neither bulkstat nor inumbers will ever generate that error +* code on their own. +*/ +typedef int (*bulkstat_one_fmt_pf )(struct xfs_ibulk *breq,const struct xfs_bulkstat *bstat);
fs/xfs/xfs_iwalk.c +1 -1
@@ -31,7 +31,7 @@* inode it finds, it calls a walk function with the relevant inode number and * a pointer to caller-provided data. The walk function can return the usual * negative error code to stop the iteration; 0 to continue the iteration; or -* XFS_IWALK_ABORT to stop the iteration. This return value is returned to the +* -ECANCELED to stop the iteration. This return value is returned to the * caller. * * Internally, we allow the walk function to do anything, which means that we
fs/xfs/xfs_iwalk.h +8 -3
@@ -6,12 +6,19 @@#ifndef __XFS_IWALK_H__#define __XFS_IWALK_H__ +/* +* Return codes for the inode/inobt walk function are 0 to continue iterating, +* and non-zero to stop iterating. Any non-zero value will be passed up to the +* iwalk or inobt_walk caller. The special value -ECANCELED can be used to +* stop iteration, as neither iwalk nor inobt_walk will ever generate that +* error code on their own. +*/ +/* Walk all inodes in the filesystem starting from @startino. */ typedef int (*xfs_iwalk_fn )(struct xfs_mount *mp,struct xfs_trans *tp,xfs_ino_t ino,void *data);/* Return values for xfs_iwalk_fn. */ #define XFS_IWALK_CONTINUE (XFS_ITER_CONTINUE) -#define XFS_IWALK_ABORT (XFS_ITER_ABORT) int xfs_iwalk(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t startino, unsigned int flags, xfs_iwalk_fn iwalk_fn,
@@ -30,8 +37,6 @@ typedef int (*xfs_inobt_walk_fn)(struct xfs_mount *mp, struct xfs_trans *tp,xfs_agnumber_t agno, const struct xfs_inobt_rec_incore *irec, void *data); -/* Return value (for xfs_inobt_walk_fn) that aborts the walk immediately. */ -#define XFS_INOBT_WALK_ABORT (XFS_IWALK_ABORT) int xfs_inobt_walk(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t startino, unsigned int flags,
Re: [PATCH 1/2] xfs: remove all *_ITER_ABORT values
Quoted 6 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Use
-ECANCELEDto signal "stop iterating" instead of these magical*_ITER_ABORTvalues, since it's duplicative.Signed-off-by Darrick J. Wong <[email protected]>
Looks good.
Signature · 2 lines
Dave Chinner [email protected]
PATCH v2 2/2 xfs: remove all *_ITER_CONTINUE values
In reply to Darrick J. WongFrom: Darrick J. Wong <[email protected]>
Iterator functions already use 0 to signal "continue iterating", so get rid of the #defines and just do it directly.
--- fs/xfs/libxfs/xfs_btree.h | 1 - fs/xfs/libxfs/xfs_rmap.c | 8 ++++---- fs/xfs/libxfs/xfs_shared.h | 3 --- fs/xfs/xfs_fsmap.c | 8 ++++---- fs/xfs/xfs_iwalk.h | 2 -- 5 files changed, 8 insertions(+), 14 deletions(-)
fs/xfs/libxfs/xfs_btree.h +0 -1
@@ -471,7 +471,6 @@ unsigned long long xfs_btree_calc_size(uint *limits, unsigned long long len);* used to stop iteration, because _query_range never generates that error * code on its own. */ -#define XFS_BTREE_QUERY_RANGE_CONTINUE (XFS_ITER_CONTINUE)/* keep iterating */ typedef int (*xfs_btree_query_range_fn )(struct xfs_btree_cur *cur,union xfs_btree_rec *rec,void *priv);
fs/xfs/libxfs/xfs_rmap.c +4 -4
@@ -253,11 +253,11 @@ xfs_rmap_find_left_neighbor_helper( rec->rm_flags);if (rec->rm_owner != info->high.rm_owner) -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ;if (!XFS_RMAP_NON_INODE_OWNER (rec->rm_owner) && !(rec->rm_flags &XFS_RMAP_BMBT_BLOCK ) && rec->rm_offset + rec->rm_blockcount -1 != info->high.rm_offset) -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; *info->irec = *rec; *info->stat =1 ;
@@ -329,12 +329,12 @@ xfs_rmap_lookup_le_range_helper( rec->rm_flags);if (rec->rm_owner != info->high.rm_owner) -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ;if (!XFS_RMAP_NON_INODE_OWNER (rec->rm_owner) && !(rec->rm_flags &XFS_RMAP_BMBT_BLOCK ) && (rec->rm_offset > info->high.rm_offset || rec->rm_offset + rec->rm_blockcount <= info->high.rm_offset)) -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; *info->irec = *rec; *info->stat =1 ;
fs/xfs/libxfs/xfs_shared.h +0 -3
@@ -177,7 +177,4 @@ struct xfs_ino_geometry { unsigned int agino_log; /* #bits for agino in inum */ }; -/* Keep iterating the data structure. */ -#define XFS_ITER_CONTINUE (0) -#endif /* __XFS_SHARED_H__ */
fs/xfs/xfs_fsmap.c +4 -4
@@ -250,7 +250,7 @@ xfs_getfsmap_helper( rec_daddr +=XFS_FSB_TO_BB (mp, rec->rm_blockcount);if (info->next_daddr < rec_daddr) info->next_daddr = rec_daddr; -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; }/* Are we just counting mappings? */
@@ -259,14 +259,14 @@ xfs_getfsmap_helper( info->head->fmh_entries++;if (info->last) -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; info->head->fmh_entries++; rec_daddr +=XFS_FSB_TO_BB (mp, rec->rm_blockcount);if (info->next_daddr < rec_daddr) info->next_daddr = rec_daddr; -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; }/*
@@ -328,7 +328,7 @@ xfs_getfsmap_helper( rec_daddr +=XFS_FSB_TO_BB (mp, rec->rm_blockcount);if (info->next_daddr < rec_daddr) info->next_daddr = rec_daddr; -return XFS_BTREE_QUERY_RANGE_CONTINUE ; +return 0 ; }/* Transform a rmapbt irec into a fsmap */
fs/xfs/xfs_iwalk.h +0 -2
@@ -17,8 +17,6 @@/* Walk all inodes in the filesystem starting from @startino. */ typedef int (*xfs_iwalk_fn )(struct xfs_mount *mp,struct xfs_trans *tp,xfs_ino_t ino,void *data); -/* Return values for xfs_iwalk_fn. */ -#define XFS_IWALK_CONTINUE (XFS_ITER_CONTINUE) int xfs_iwalk(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t startino, unsigned int flags, xfs_iwalk_fn iwalk_fn,
Re: [PATCH 2/2] xfs: remove all *_ITER_CONTINUE values
Quoted 6 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Iterator functions already use 0 to signal "continue iterating", so get rid of the #defines and just do it directly.
Signed-off-by Darrick J. Wong <[email protected]>
Looks good.
Signature · 2 lines
Dave Chinner [email protected]