On Tue, May 19, 2020 at 06:45:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> The incore inode walk code passes a flags argument and a pointer from
> the xfs_inode_ag_iterator caller all the way to the iteration function.
> We can reduce the function complexity by passing flags through the
> private pointer.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_icache.c | 38 ++++++++++++++------------------------
> fs/xfs/xfs_icache.h | 4 ++--
> fs/xfs/xfs_qm_syscalls.c | 25 +++++++++++++++++--------
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index e716b19879c6..87b98bfdf27d 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -790,9 +790,7 @@ STATIC int
> xfs_inode_ag_walk(
> struct xfs_mount *mp,
> struct xfs_perag *pag,
> - int (*execute)(struct xfs_inode *ip, int flags,
> - void *args),
> - int flags,
> + int (*execute)(struct xfs_inode *ip, void *args),
> void *args,
> int tag,
> int iter_flags)
> @@ -868,7 +866,7 @@ xfs_inode_ag_walk(
> if ((iter_flags & XFS_AGITER_INEW_WAIT) &&
> xfs_iflags_test(batch[i], XFS_INEW))
> xfs_inew_wait(batch[i]);
> - error = execute(batch[i], flags, args);
> + error = execute(batch[i], args);
> xfs_irele(batch[i]);
> if (error == -EAGAIN) {
> skipped++;
> @@ -972,9 +970,7 @@ int
> xfs_inode_ag_iterator(
> struct xfs_mount *mp,
> int iter_flags,
> - int (*execute)(struct xfs_inode *ip, int flags,
> - void *args),
> - int flags,
> + int (*execute)(struct xfs_inode *ip, void *args),
> void *args,
> int tag)
> {
> @@ -986,7 +982,7 @@ xfs_inode_ag_iterator(
> ag = 0;
> while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
> ag = pag->pag_agno + 1;
> - error = xfs_inode_ag_walk(mp, pag, execute, flags, args, tag,
> + error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
> iter_flags);
> xfs_perag_put(pag);
> if (error) {
> @@ -1443,12 +1439,14 @@ xfs_inode_match_id_union(
> STATIC int
> xfs_inode_free_eofblocks(
> struct xfs_inode *ip,
> - int flags,
> void *args)
> {
> - int ret = 0;
> - struct xfs_eofblocks *eofb = args;
> - int match;
> + struct xfs_eofblocks *eofb = args;
> + bool wait;
> + int match;
> + int ret = 0;
> +
> + wait = (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC));
No need for the outer braces.
> @@ -1484,7 +1481,7 @@ xfs_inode_free_eofblocks(
> * scanner moving and revisit the inode in a subsequent pass.
> */
> if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
> - if (flags & SYNC_WAIT)
> + if (wait)
> ret = -EAGAIN;
> return ret;
Just me, but I'd prefer an explicit:
if (wait)
return -EAGAIN;
return 0;
here. Not really new in this patch, but if you touch this area anyway..
> index a9460bdcca87..571ecb17b3bf 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -726,12 +726,17 @@ xfs_qm_scall_getquota_next(
> return error;
> }
>
> +struct xfs_dqrele {
> + uint flags;
> +};
> + struct xfs_dqrele dqr = {
> + .flags = flags,
> + };
Instead of a new structure we could just take the address of flags and
pass that to simplify the code a bit.
