On Thu, Aug 29, 2019 at 02:02:25PM -0700, Darrick J. Wong wrote:
> On Thu, Aug 29, 2019 at 08:47:07PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Factor out the code that adds a data block to a directory from
> > xfs_dir2_node_addname_int(). This makes the code flow cleaner and
> > more obvious and provides clear isolation of upcoming optimsations.
> >
> > Signed-off-By: Dave Chinner <dchinner@xxxxxxxxxx>
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_dir2_node.c | 324 +++++++++++++++++-----------------
> > 1 file changed, 158 insertions(+), 166 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> > index e40986cc0759..cc1f1c505a2b 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_node.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> > @@ -1608,6 +1608,129 @@ xfs_dir2_leafn_unbalance(
> > xfs_dir3_leaf_check(dp, drop_blk->bp);
> > }
> >
> > +/*
> > + * Add a new data block to the directory at the free space index that the caller
> > + * has specified.
> > + */
> > +static int
> > +xfs_dir2_node_add_datablk(
> > + struct xfs_da_args *args,
> > + struct xfs_da_state_blk *fblk,
> > + xfs_dir2_db_t *dbno,
> > + struct xfs_buf **dbpp,
> > + struct xfs_buf **fbpp,
> > + int *findex)
> > +{
> > + struct xfs_inode *dp = args->dp;
> > + struct xfs_trans *tp = args->trans;
> > + struct xfs_mount *mp = dp->i_mount;
> > + struct xfs_dir3_icfree_hdr freehdr;
> > + struct xfs_dir2_data_free *bf;
> > + struct xfs_dir2_data_hdr *hdr;
> > + struct xfs_dir2_free *free = NULL;
> > + xfs_dir2_db_t fbno;
> > + struct xfs_buf *fbp;
> > + struct xfs_buf *dbp;
> > + __be16 *bests = NULL;
> > + int error;
> > +
> > + /* Not allowed to allocate, return failure. */
> > + if ((args->op_flags & XFS_DA_OP_JUSTCHECK) || args->total == 0)
> > + return -ENOSPC;
> > +
> > + /* Allocate and initialize the new data block. */
> > + error = xfs_dir2_grow_inode(args, XFS_DIR2_DATA_SPACE, dbno);
> > + if (error)
> > + return error;
> > + error = xfs_dir3_data_init(args, *dbno, &dbp);
> > + if (error)
> > + return error;
> > +
> > + /*
> > + * Get the freespace block corresponding to the data block
> > + * that was just allocated.
> > + */
> > + fbno = dp->d_ops->db_to_fdb(args->geo, *dbno);
> > + error = xfs_dir2_free_try_read(tp, dp,
> > + xfs_dir2_db_to_da(args->geo, fbno), &fbp);
> > + if (error)
> > + return error;
> > +
> > + /*
> > + * If there wasn't a freespace block, the read will
> > + * return a NULL fbp. Allocate and initialize a new one.
> > + */
> > + if (!fbp) {
> > + error = xfs_dir2_grow_inode(args, XFS_DIR2_FREE_SPACE, &fbno);
> > + if (error)
> > + return error;
> > +
> > + if (dp->d_ops->db_to_fdb(args->geo, *dbno) != fbno) {
>
> As a straight "cut this huge function into smaller pieces, no functional
> changes" patch I guess this is fine, but ... when can this happen?
No idea. We're growing the free space segment, and it's checking
that we allocated a new block over the free block index for the
data segment block we just allocated. It's a sanity check more than
anything, I think, because if we have a hole in the free block index
for a give data block bad things are going to happen later...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
