Re: [PATCH 12/16] xfs: reduce direct usage of mp->m_dir_geo

Dave Chinner
2014-05-27T23:53:16Z

On Tue, May 27, 2014 at 11:59:57AM -0400, Brian Foster wrote:
> On Fri, May 23, 2014 at 10:03:48AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > There are many places in the directory code were we don't pass the
> > args into and so have to extract the geometry direct from the mount
> > structure. Push the args or the geometry into these leaf functions
> > so that we don't need to grab it from the struct xfs_mount.
> >
> > This, in turn, brings use to the point where directory geometry is
> > no longer a property of the struct xfs_mount; it is not a global
> > property anymore, and hence we can start to consider per-directory
> > configuration of physical geometries.
> >
> > Start by converting the xfs_dir_isblock/leaf code - pass in the
> > xfs_da_args and convert the readdir code to use xfs_da_args like
> > the rest of the directory code to pass information around.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
.....
> > STATIC int
> > xfs_dir2_leaf_readbuf(
> > - struct xfs_inode *dp,
> > + struct xfs_da_args *args,
> > size_t bufsize,
> > struct xfs_dir2_leaf_map_info *mip,
> > xfs_dir2_off_t *curoff,
> > struct xfs_buf **bpp)
> > {
> > - struct xfs_mount *mp = dp->i_mount;
> > + struct xfs_inode *dp = args->dp;
> > struct xfs_buf *bp = *bpp;
> > struct xfs_bmbt_irec *map = mip->map;
> > struct blk_plug plug;
> > @@ -304,7 +297,7 @@ xfs_dir2_leaf_readbuf(
> > int length;
> > int i;
> > int j;
> > - struct xfs_da_geometry *geo = mp->m_dir_geo;
> > + struct xfs_da_geometry *geo = args->geo;
>
> I notice that we remove the local geo pointer in some of the immediately
> previous fixups. Here we keep it...
>
> >
> > /*
> > * If we have a buffer, we need to release it and
> > @@ -314,7 +307,7 @@ xfs_dir2_leaf_readbuf(
> > if (bp) {
> > xfs_trans_brelse(NULL, bp);
> > bp = NULL;
> > - mip->map_blocks -= geo->fsbcount;
> > + mip->map_blocks -= args->geo->fsbcount;
>
> ... convert some calls to use args...
>
> > /*
> > * Loop to get rid of the extents for the
> > * directory block.
> > @@ -338,8 +331,7 @@ xfs_dir2_leaf_readbuf(
> > /*
> > * Recalculate the readahead blocks wanted.
> > */
> > - mip->ra_want = howmany(bufsize + geo->blksize,
> > - mp->m_sb.sb_blocksize) - 1;
> > + mip->ra_want = howmany(bufsize + geo->blksize, (1 << geo->fsblog)) - 1;
> > ASSERT(mip->ra_want >= 0);
>
> ... and not others. So we should probably at least be consistent within
> function. :) Otherwise this patch looks good to me.

Yup, brain fart. i converted each function in the entire file to use
a local geo variable....

-Dave
--
Dave Chinner
david@fromorbit.com