Re: [PATCH 08/14] xfs: remove indirect calls from xfs_inode_walk{,_ag}
From: Dave Chinner <david@fromorbit.com>
Date: 2021-06-02 02:00:51
On Tue, Jun 01, 2021 at 05:53:16PM -0700, Darrick J. Wong wrote:
From: Darrick J. Wong <djwong@kernel.org> It turns out that there is a 1:1 mapping between the execute and goal parameters that are passed to xfs_inode_walk_ag: xfs_blockgc_scan_inode <=> XFS_ICWALK_BLOCKGC xfs_dqrele_inode <=> XFS_ICWALK_DQRELE Because of this exact correspondence, we don't need the execute function pointer and can replace it with a direct call. For the price of a forward static declaration, we can eliminate the indirect function call. This likely has a negligible impact on performance (since the execute function runs transactions), but it also simplifies the function signature. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- fs/xfs/xfs_icache.c | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-)
Overall looks good.
quoted hunk ↗ jump to hunk
@@ -1777,7 +1772,14 @@ xfs_inode_walk_ag( for (i = 0; i < nr_found; i++) { if (!batch[i]) continue; - error = execute(batch[i], args); + switch (goal) { + case XFS_ICWALK_DQRELE: + error = xfs_dqrele_inode(batch[i], args); + break; + case XFS_ICWALK_BLOCKGC: + error = xfs_blockgc_scan_inode(batch[i], args); + break; + } xfs_irele(batch[i]);
I can't help but think that this should be wrapped up in a function
like xfs_icwalk_grab(), especially as we add the reclaim case to
this later on. Perhaps:
static int
xfs_icwalk_process_inode(
struct xfs_inode *ip,
enum xfs_icwalk_goal goal,
void *args)
{
int error = -EINVAL;
switch (goal) {
case XFS_ICWALK_DQRELE:
error = xfs_dqrele_inode(ip, args);
break;
case XFS_ICWALK_BLOCKGC:
error = xfs_blockgc_scan_inode(ip, args);
break;
}
xfs_irele(ip);
return error;
}
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com