xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 12/16] xfs: reduce direct usage of mp->m_dir_geo
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 30 May 2014 15:05:24 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1401257083-13433-13-git-send-email-david@xxxxxxxxxxxxx>
References: <1401257083-13433-1-git-send-email-david@xxxxxxxxxxxxx> <1401257083-13433-13-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Wed, May 28, 2014 at 04:04:39PM +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>
> ---

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_dir2.c         | 50 +++++++++++++---------------
>  fs/xfs/xfs_dir2.h         |  4 +--
>  fs/xfs/xfs_dir2_readdir.c | 85 
> ++++++++++++++++++++++-------------------------
>  3 files changed, 65 insertions(+), 74 deletions(-)
> 
> diff --git a/fs/xfs/xfs_dir2.c b/fs/xfs/xfs_dir2.c
> index f62d2e5..79670cd 100644
> --- a/fs/xfs/xfs_dir2.c
> +++ b/fs/xfs/xfs_dir2.c
> @@ -282,7 +282,7 @@ xfs_dir_createname(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isblock(dp, &v);
> +     rval = xfs_dir2_isblock(args, &v);
>       if (rval)
>               goto out_free;
>       if (v) {
> @@ -290,7 +290,7 @@ xfs_dir_createname(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isleaf(dp, &v);
> +     rval = xfs_dir2_isleaf(args, &v);
>       if (rval)
>               goto out_free;
>       if (v)
> @@ -375,7 +375,7 @@ xfs_dir_lookup(
>               goto out_check_rval;
>       }
>  
> -     rval = xfs_dir2_isblock(dp, &v);
> +     rval = xfs_dir2_isblock(args, &v);
>       if (rval)
>               goto out_free;
>       if (v) {
> @@ -383,7 +383,7 @@ xfs_dir_lookup(
>               goto out_check_rval;
>       }
>  
> -     rval = xfs_dir2_isleaf(dp, &v);
> +     rval = xfs_dir2_isleaf(args, &v);
>       if (rval)
>               goto out_free;
>       if (v)
> @@ -448,7 +448,7 @@ xfs_dir_removename(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isblock(dp, &v);
> +     rval = xfs_dir2_isblock(args, &v);
>       if (rval)
>               goto out_free;
>       if (v) {
> @@ -456,7 +456,7 @@ xfs_dir_removename(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isleaf(dp, &v);
> +     rval = xfs_dir2_isleaf(args, &v);
>       if (rval)
>               goto out_free;
>       if (v)
> @@ -513,7 +513,7 @@ xfs_dir_replace(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isblock(dp, &v);
> +     rval = xfs_dir2_isblock(args, &v);
>       if (rval)
>               goto out_free;
>       if (v) {
> @@ -521,7 +521,7 @@ xfs_dir_replace(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isleaf(dp, &v);
> +     rval = xfs_dir2_isleaf(args, &v);
>       if (rval)
>               goto out_free;
>       if (v)
> @@ -573,7 +573,7 @@ xfs_dir_canenter(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isblock(dp, &v);
> +     rval = xfs_dir2_isblock(args, &v);
>       if (rval)
>               goto out_free;
>       if (v) {
> @@ -581,7 +581,7 @@ xfs_dir_canenter(
>               goto out_free;
>       }
>  
> -     rval = xfs_dir2_isleaf(dp, &v);
> +     rval = xfs_dir2_isleaf(args, &v);
>       if (rval)
>               goto out_free;
>       if (v)
> @@ -649,18 +649,16 @@ xfs_dir2_grow_inode(
>   */
>  int
>  xfs_dir2_isblock(
> -     xfs_inode_t     *dp,
> -     int             *vp)            /* out: 1 is block, 0 is not block */
> +     struct xfs_da_args      *args,
> +     int                     *vp)    /* out: 1 is block, 0 is not block */
>  {
> -     xfs_fileoff_t   last;           /* last file offset */
> -     xfs_mount_t     *mp;
> -     int             rval;
> +     xfs_fileoff_t           last;   /* last file offset */
> +     int                     rval;
>  
> -     mp = dp->i_mount;
> -     if ((rval = xfs_bmap_last_offset(dp, &last, XFS_DATA_FORK)))
> +     if ((rval = xfs_bmap_last_offset(args->dp, &last, XFS_DATA_FORK)))
>               return rval;
> -     rval = XFS_FSB_TO_B(mp, last) == mp->m_dir_geo->blksize;
> -     ASSERT(rval == 0 || dp->i_d.di_size == mp->m_dir_geo->blksize);
> +     rval = XFS_FSB_TO_B(args->dp->i_mount, last) == args->geo->blksize;
> +     ASSERT(rval == 0 || args->dp->i_d.di_size == args->geo->blksize);
>       *vp = rval;
>       return 0;
>  }
> @@ -670,17 +668,15 @@ xfs_dir2_isblock(
>   */
>  int
>  xfs_dir2_isleaf(
> -     xfs_inode_t     *dp,
> -     int             *vp)            /* out: 1 is leaf, 0 is not leaf */
> +     struct xfs_da_args      *args,
> +     int                     *vp)    /* out: 1 is block, 0 is not block */
>  {
> -     xfs_fileoff_t   last;           /* last file offset */
> -     xfs_mount_t     *mp;
> -     int             rval;
> +     xfs_fileoff_t           last;   /* last file offset */
> +     int                     rval;
>  
> -     mp = dp->i_mount;
> -     if ((rval = xfs_bmap_last_offset(dp, &last, XFS_DATA_FORK)))
> +     if ((rval = xfs_bmap_last_offset(args->dp, &last, XFS_DATA_FORK)))
>               return rval;
> -     *vp = last == mp->m_dir_geo->leafblk + (1 << mp->m_sb.sb_dirblklog);
> +     *vp = last == args->geo->leafblk + args->geo->fsbcount;
>       return 0;
>  }
>  
> diff --git a/fs/xfs/xfs_dir2.h b/fs/xfs/xfs_dir2.h
> index 9b709ae..1292e78 100644
> --- a/fs/xfs/xfs_dir2.h
> +++ b/fs/xfs/xfs_dir2.h
> @@ -146,8 +146,8 @@ extern int xfs_dir2_sf_to_block(struct xfs_da_args *args);
>  /*
>   * Interface routines used by userspace utilities
>   */
> -extern int xfs_dir2_isblock(struct xfs_inode *dp, int *r);
> -extern int xfs_dir2_isleaf(struct xfs_inode *dp, int *r);
> +extern int xfs_dir2_isblock(struct xfs_da_args *args, int *r);
> +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);
>  
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index fa393d5..ca1f43c 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -76,28 +76,25 @@ const unsigned char xfs_mode_to_ftype[S_IFMT >> S_SHIFT] 
> = {
>  
>  STATIC int
>  xfs_dir2_sf_getdents(
> -     xfs_inode_t             *dp,            /* incore directory inode */
> +     struct xfs_da_args      *args,
>       struct dir_context      *ctx)
>  {
>       int                     i;              /* shortform entry number */
> -     xfs_mount_t             *mp;            /* filesystem mount point */
> +     struct xfs_inode        *dp = args->dp; /* incore directory inode */
>       xfs_dir2_dataptr_t      off;            /* current entry's offset */
>       xfs_dir2_sf_entry_t     *sfep;          /* shortform directory entry */
>       xfs_dir2_sf_hdr_t       *sfp;           /* shortform structure */
>       xfs_dir2_dataptr_t      dot_offset;
>       xfs_dir2_dataptr_t      dotdot_offset;
>       xfs_ino_t               ino;
> -     struct xfs_da_geometry  *geo;
> -
> -     mp = dp->i_mount;
> -     geo = mp->m_dir_geo;
> +     struct xfs_da_geometry  *geo = args->geo;
>  
>       ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
>       /*
>        * Give up if the directory is way too short.
>        */
>       if (dp->i_d.di_size < offsetof(xfs_dir2_sf_hdr_t, parent)) {
> -             ASSERT(XFS_FORCED_SHUTDOWN(mp));
> +             ASSERT(XFS_FORCED_SHUTDOWN(dp->i_mount));
>               return XFS_ERROR(EIO);
>       }
>  
> @@ -163,13 +160,13 @@ xfs_dir2_sf_getdents(
>               filetype = dp->d_ops->sf_get_ftype(sfep);
>               ctx->pos = off & 0x7fffffff;
>               if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
> -                         xfs_dir3_get_dtype(mp, filetype)))
> +                         xfs_dir3_get_dtype(dp->i_mount, filetype)))
>                       return 0;
>               sfep = dp->d_ops->sf_nextentry(sfp, sfep);
>       }
>  
>       ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
> -                     0x7fffffff;
> +                                                             0x7fffffff;
>       return 0;
>  }
>  
> @@ -178,9 +175,10 @@ xfs_dir2_sf_getdents(
>   */
>  STATIC int
>  xfs_dir2_block_getdents(
> -     xfs_inode_t             *dp,            /* incore inode */
> +     struct xfs_da_args      *args,
>       struct dir_context      *ctx)
>  {
> +     struct xfs_inode        *dp = args->dp; /* incore directory inode */
>       xfs_dir2_data_hdr_t     *hdr;           /* block header */
>       struct xfs_buf          *bp;            /* buffer for block */
>       xfs_dir2_block_tail_t   *btp;           /* block tail */
> @@ -188,14 +186,11 @@ xfs_dir2_block_getdents(
>       xfs_dir2_data_unused_t  *dup;           /* block unused entry */
>       char                    *endptr;        /* end of the data entries */
>       int                     error;          /* error return value */
> -     xfs_mount_t             *mp;            /* filesystem mount point */
>       char                    *ptr;           /* current data entry */
>       int                     wantoff;        /* starting block offset */
>       xfs_off_t               cook;
> -     struct xfs_da_geometry  *geo;
> +     struct xfs_da_geometry  *geo = args->geo;
>  
> -     mp = dp->i_mount;
> -     geo = mp->m_dir_geo;
>       /*
>        * If the block number in the offset is out of range, we're done.
>        */
> @@ -258,7 +253,7 @@ xfs_dir2_block_getdents(
>                */
>               if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
>                           be64_to_cpu(dep->inumber),
> -                         xfs_dir3_get_dtype(mp, filetype))) {
> +                         xfs_dir3_get_dtype(dp->i_mount, filetype))) {
>                       xfs_trans_brelse(NULL, bp);
>                       return 0;
>               }
> @@ -269,7 +264,7 @@ xfs_dir2_block_getdents(
>        * Set the offset to a non-existent block 1 and return.
>        */
>       ctx->pos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk + 1, 0) &
> -                     0x7fffffff;
> +                                                             0x7fffffff;
>       xfs_trans_brelse(NULL, bp);
>       return 0;
>  }
> @@ -290,13 +285,13 @@ struct xfs_dir2_leaf_map_info {
>  
>  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 +299,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;
>  
>       /*
>        * If we have a buffer, we need to release it and
> @@ -338,8 +333,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);
>  
>       /*
> @@ -411,8 +405,8 @@ xfs_dir2_leaf_readbuf(
>       mip->curdb = xfs_dir2_da_to_db(geo, map->br_startoff);
>       error = xfs_dir3_data_read(NULL, dp, map->br_startoff,
>                       map->br_blockcount >= geo->fsbcount ?
> -                         XFS_FSB_TO_DADDR(mp, map->br_startblock) : -1, &bp);
> -
> +                         XFS_FSB_TO_DADDR(dp->i_mount, map->br_startblock) :
> +                         -1, &bp);
>       /*
>        * Should just skip over the data block instead of giving up.
>        */
> @@ -441,7 +435,7 @@ xfs_dir2_leaf_readbuf(
>                   map[mip->ra_index].br_blockcount >= geo->fsbcount) {
>                       xfs_dir3_data_readahead(dp,
>                               map[mip->ra_index].br_startoff + mip->ra_offset,
> -                             XFS_FSB_TO_DADDR(mp,
> +                             XFS_FSB_TO_DADDR(dp->i_mount,
>                                       map[mip->ra_index].br_startblock +
>                                                       mip->ra_offset));
>                       mip->ra_current = i;
> @@ -493,23 +487,23 @@ out:
>   */
>  STATIC int
>  xfs_dir2_leaf_getdents(
> -     xfs_inode_t             *dp,            /* incore directory inode */
> +     struct xfs_da_args      *args,
>       struct dir_context      *ctx,
>       size_t                  bufsize)
>  {
> +     struct xfs_inode        *dp = args->dp;
>       struct xfs_buf          *bp = NULL;     /* data block buffer */
>       xfs_dir2_data_hdr_t     *hdr;           /* data block header */
>       xfs_dir2_data_entry_t   *dep;           /* data entry */
>       xfs_dir2_data_unused_t  *dup;           /* unused entry */
>       int                     error = 0;      /* error return value */
>       int                     length;         /* temporary length value */
> -     xfs_mount_t             *mp;            /* filesystem mount point */
>       int                     byteoff;        /* offset in current block */
>       xfs_dir2_off_t          curoff;         /* current overall offset */
>       xfs_dir2_off_t          newoff;         /* new curoff after new blk */
>       char                    *ptr = NULL;    /* pointer to current data */
>       struct xfs_dir2_leaf_map_info *map_info;
> -     struct xfs_da_geometry  *geo;
> +     struct xfs_da_geometry  *geo = args->geo;
>  
>       /*
>        * If the offset is at or past the largest allowed value,
> @@ -518,15 +512,12 @@ xfs_dir2_leaf_getdents(
>       if (ctx->pos >= XFS_DIR2_MAX_DATAPTR)
>               return 0;
>  
> -     mp = dp->i_mount;
> -     geo = mp->m_dir_geo;
> -
>       /*
>        * Set up to bmap a number of blocks based on the caller's
>        * buffer size, the directory block size, and the filesystem
>        * block size.
>        */
> -     length = howmany(bufsize + geo->blksize, mp->m_sb.sb_blocksize);
> +     length = howmany(bufsize + geo->blksize, (1 << geo->fsblog));
>       map_info = kmem_zalloc(offsetof(struct xfs_dir2_leaf_map_info, map) +
>                               (length * sizeof(struct xfs_bmbt_irec)),
>                              KM_SLEEP | KM_NOFS);
> @@ -558,7 +549,7 @@ xfs_dir2_leaf_getdents(
>                */
>               if (!bp || ptr >= (char *)bp->b_addr + geo->blksize) {
>  
> -                     error = xfs_dir2_leaf_readbuf(dp, bufsize, map_info,
> +                     error = xfs_dir2_leaf_readbuf(args, bufsize, map_info,
>                                                     &curoff, &bp);
>                       if (error || !map_info->map_valid)
>                               break;
> @@ -566,7 +557,7 @@ xfs_dir2_leaf_getdents(
>                       /*
>                        * Having done a read, we need to set a new offset.
>                        */
> -                     newoff = xfs_dir2_db_off_to_byte(mp->m_dir_geo,
> +                     newoff = xfs_dir2_db_off_to_byte(geo,
>                                                        map_info->curdb, 0);
>                       /*
>                        * Start of the current block.
> @@ -585,7 +576,7 @@ xfs_dir2_leaf_getdents(
>                        * Find our position in the block.
>                        */
>                       ptr = (char *)dp->d_ops->data_entry_p(hdr);
> -                     byteoff = xfs_dir2_byte_to_off(mp->m_dir_geo, curoff);
> +                     byteoff = xfs_dir2_byte_to_off(geo, curoff);
>                       /*
>                        * Skip past the header.
>                        */
> @@ -644,7 +635,7 @@ xfs_dir2_leaf_getdents(
>               ctx->pos = xfs_dir2_byte_to_dataptr(curoff) & 0x7fffffff;
>               if (!dir_emit(ctx, (char *)dep->name, dep->namelen,
>                           be64_to_cpu(dep->inumber),
> -                         xfs_dir3_get_dtype(mp, filetype)))
> +                         xfs_dir3_get_dtype(dp->i_mount, filetype)))
>                       break;
>  
>               /*
> @@ -674,13 +665,14 @@ xfs_dir2_leaf_getdents(
>   */
>  int
>  xfs_readdir(
> -     xfs_inode_t     *dp,
> -     struct dir_context *ctx,
> -     size_t          bufsize)
> +     struct xfs_inode        *dp,
> +     struct dir_context      *ctx,
> +     size_t                  bufsize)
>  {
> -     int             rval;           /* return value */
> -     int             v;              /* type-checking value */
> -     uint            lock_mode;
> +     struct xfs_da_args      args = {0};
> +     int                     rval;
> +     int                     v;
> +     uint                    lock_mode;
>  
>       trace_xfs_readdir(dp);
>  
> @@ -690,15 +682,18 @@ xfs_readdir(
>       ASSERT(S_ISDIR(dp->i_d.di_mode));
>       XFS_STATS_INC(xs_dir_getdents);
>  
> +     args.dp = dp;
> +     args.geo = dp->i_mount->m_dir_geo;
> +
>       lock_mode = xfs_ilock_data_map_shared(dp);
>       if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
> -             rval = xfs_dir2_sf_getdents(dp, ctx);
> -     else if ((rval = xfs_dir2_isblock(dp, &v)))
> +             rval = xfs_dir2_sf_getdents(&args, ctx);
> +     else if ((rval = xfs_dir2_isblock(&args, &v)))
>               ;
>       else if (v)
> -             rval = xfs_dir2_block_getdents(dp, ctx);
> +             rval = xfs_dir2_block_getdents(&args, ctx);
>       else
> -             rval = xfs_dir2_leaf_getdents(dp, ctx, bufsize);
> +             rval = xfs_dir2_leaf_getdents(&args, ctx, bufsize);
>       xfs_iunlock(dp, lock_mode);
>  
>       return rval;
> -- 
> 1.9.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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