On Fri, Nov 04, 2016 at 06:20:22PM -0500, Eric Sandeen wrote: > On 11/4/16 6:09 PM, Darrick J. Wong wrote: > > On Fri, Nov 04, 2016 at 05:58:57PM -0500, Eric Sandeen wrote: > >> On 11/4/16 1:32 PM, Darrick J. Wong wrote: > >>> Refactor the implementations of xfs_dir2_data_freescan into a > >>> routine that takes the raw directory block parameters and > >>> a second function that figures out the raw parameters from the > >>> directory inode. This enables us to use the exact same code > >>> for both userspace and the kernel, since repair knows exactly > >>> which directory block geometry parameters it needs. > >>> > >>> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > >> > >> seems fine tho I can't say I like the new function name; what > >> does "_hdr" mean? They both take a *hdr ... > >> > >> maybe _xfs_dir2_data_freescan or xfs_dir2_data_freescan_int? > > > > I'll change it to xfs_dir2_data_freescan_geo(). > > it gets both geo and ops, both derived from dp ... > > see i.e. xfs_btree_init_block, xfs_rtmodify_summary etc for > examples of a simple args translation calling an "*_int()" > (internal?) function. That's pretty common in the xfs code. Ok I'll change it. --D > > -Eric > > > --D > > > >> > >> *shrug* > >> > >> -Eric > >> > >>> --- > >>> fs/xfs/libxfs/xfs_dir2.h | 3 +++ > >>> fs/xfs/libxfs/xfs_dir2_data.c | 24 +++++++++++++++++------- > >>> 2 files changed, 20 insertions(+), 7 deletions(-) > >>> > >>> > >>> diff --git a/fs/xfs/libxfs/xfs_dir2.h b/fs/xfs/libxfs/xfs_dir2.h > >>> index becc926..42456de 100644 > >>> --- a/fs/xfs/libxfs/xfs_dir2.h > >>> +++ b/fs/xfs/libxfs/xfs_dir2.h > >>> @@ -157,6 +157,9 @@ extern int xfs_dir2_isleaf(struct xfs_da_args *args, int *r); > >>> extern int xfs_dir2_shrink_inode(struct xfs_da_args *args, xfs_dir2_db_t db, > >>> struct xfs_buf *bp); > >>> > >>> +extern void xfs_dir2_data_freescan_hdr(struct xfs_da_geometry *geo, > >>> + const struct xfs_dir_ops *ops, > >>> + struct xfs_dir2_data_hdr *hdr, int *loghead); > >>> extern void xfs_dir2_data_freescan(struct xfs_inode *dp, > >>> struct xfs_dir2_data_hdr *hdr, int *loghead); > >>> extern void xfs_dir2_data_log_entry(struct xfs_da_args *args, > >>> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c > >>> index 725fc78..1729eb2 100644 > >>> --- a/fs/xfs/libxfs/xfs_dir2_data.c > >>> +++ b/fs/xfs/libxfs/xfs_dir2_data.c > >>> @@ -505,8 +505,9 @@ xfs_dir2_data_freeremove( > >>> * Given a data block, reconstruct its bestfree map. > >>> */ > >>> void > >>> -xfs_dir2_data_freescan( > >>> - struct xfs_inode *dp, > >>> +xfs_dir2_data_freescan_hdr( > >>> + struct xfs_da_geometry *geo, > >>> + const struct xfs_dir_ops *ops, > >>> struct xfs_dir2_data_hdr *hdr, > >>> int *loghead) > >>> { > >>> @@ -516,7 +517,6 @@ xfs_dir2_data_freescan( > >>> struct xfs_dir2_data_free *bf; > >>> char *endp; /* end of block's data */ > >>> char *p; /* current entry pointer */ > >>> - struct xfs_da_geometry *geo = dp->i_mount->m_dir_geo; > >>> > >>> ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) || > >>> hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) || > >>> @@ -526,13 +526,13 @@ xfs_dir2_data_freescan( > >>> /* > >>> * Start by clearing the table. > >>> */ > >>> - bf = dp->d_ops->data_bestfree_p(hdr); > >>> + bf = ops->data_bestfree_p(hdr); > >>> memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT); > >>> *loghead = 1; > >>> /* > >>> * Set up pointers. > >>> */ > >>> - p = (char *)dp->d_ops->data_entry_p(hdr); > >>> + p = (char *)ops->data_entry_p(hdr); > >>> if (hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) || > >>> hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC)) { > >>> btp = xfs_dir2_block_tail_p(geo, hdr); > >>> @@ -559,12 +559,22 @@ xfs_dir2_data_freescan( > >>> else { > >>> dep = (xfs_dir2_data_entry_t *)p; > >>> ASSERT((char *)dep - (char *)hdr == > >>> - be16_to_cpu(*dp->d_ops->data_entry_tag_p(dep))); > >>> - p += dp->d_ops->data_entsize(dep->namelen); > >>> + be16_to_cpu(*ops->data_entry_tag_p(dep))); > >>> + p += ops->data_entsize(dep->namelen); > >>> } > >>> } > >>> } > >>> > >>> +void > >>> +xfs_dir2_data_freescan( > >>> + struct xfs_inode *dp, > >>> + struct xfs_dir2_data_hdr *hdr, > >>> + int *loghead) > >>> +{ > >>> + return xfs_dir2_data_freescan_hdr(dp->i_mount->m_dir_geo, dp->d_ops, > >>> + hdr, loghead); > >>> +} > >>> + > >>> /* > >>> * Initialize a data block at the given block number in the directory. > >>> * Give back the buffer for the created block. > >>> > >>> -- > >>> 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 > > -- 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