xfs
[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 12/16] xfs: reduce direct usage of mp->m_dir_geo
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 28 May 2014 09:53:07 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140527155957.GD11265@xxxxxxxxxxxxxx>
References: <1400803432-20048-1-git-send-email-david@xxxxxxxxxxxxx> <1400803432-20048-13-git-send-email-david@xxxxxxxxxxxxx> <20140527155957.GD11265@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
.....
> >  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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>