From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 05/16] xfs: convert directory dablk conversion to xfs_da_geometry
Date: Fri, 23 May 2014 15:06:22 -0400 [thread overview]
Message-ID: <20140523190622.GF8343@laptop.bfoster> (raw)
In-Reply-To: <1400803432-20048-6-git-send-email-david@fromorbit.com>
On Fri, May 23, 2014 at 10:03:41AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_da_btree.h | 15 +++++++--------
> fs/xfs/xfs_dir2.c | 4 ++--
> fs/xfs/xfs_dir2_data.c | 4 ++--
> fs/xfs/xfs_dir2_leaf.c | 21 +++++++++++----------
> fs/xfs/xfs_dir2_node.c | 41 +++++++++++++++++++++++------------------
> fs/xfs/xfs_dir2_readdir.c | 6 +++---
> 6 files changed, 48 insertions(+), 43 deletions(-)
>
> diff --git a/fs/xfs/xfs_da_btree.h b/fs/xfs/xfs_da_btree.h
> index e9496a9..8db83fc 100644
> --- a/fs/xfs/xfs_da_btree.h
> +++ b/fs/xfs/xfs_da_btree.h
> @@ -232,9 +232,9 @@ xfs_dir2_db_off_to_byte(struct xfs_da_geometry *geo, xfs_dir2_db_t db,
> * Convert block (DB) to block (dablk)
> */
> static inline xfs_dablk_t
> -xfs_dir2_db_to_da(struct xfs_mount *mp, xfs_dir2_db_t db)
> +xfs_dir2_db_to_da(struct xfs_da_geometry *geo, xfs_dir2_db_t db)
> {
> - return (xfs_dablk_t)(db << mp->m_sb.sb_dirblklog);
> + return (xfs_dablk_t)(db << (geo->blklog - geo->fsblog));
> }
>
> /*
> @@ -243,7 +243,7 @@ xfs_dir2_db_to_da(struct xfs_mount *mp, xfs_dir2_db_t db)
> static inline xfs_dablk_t
> xfs_dir2_byte_to_da(struct xfs_mount *mp, xfs_dir2_off_t by)
> {
> - return xfs_dir2_db_to_da(mp, xfs_dir2_byte_to_db(mp, by));
> + return xfs_dir2_db_to_da(mp->m_dir_geo, xfs_dir2_byte_to_db(mp, by));
> }
>
> /*
> @@ -261,19 +261,18 @@ xfs_dir2_db_off_to_dataptr(struct xfs_mount *mp, xfs_dir2_db_t db,
> * Convert block (dablk) to block (DB)
> */
> static inline xfs_dir2_db_t
> -xfs_dir2_da_to_db(struct xfs_mount *mp, xfs_dablk_t da)
> +xfs_dir2_da_to_db(struct xfs_da_geometry *geo, xfs_dablk_t da)
> {
> - return (xfs_dir2_db_t)(da >> mp->m_sb.sb_dirblklog);
> + return (xfs_dir2_db_t)(da >> (geo->blklog - geo->fsblog));
> }
>
> /*
> * Convert block (dablk) to byte offset in space
> */
> static inline xfs_dir2_off_t
> -xfs_dir2_da_to_byte(struct xfs_mount *mp, xfs_dablk_t da)
> +xfs_dir2_da_to_byte(struct xfs_da_geometry *geo, xfs_dablk_t da)
> {
> - return xfs_dir2_db_off_to_byte(mp->m_dir_geo,
> - xfs_dir2_da_to_db(mp, da), 0);
> + return xfs_dir2_db_off_to_byte(geo, xfs_dir2_da_to_db(geo, da), 0);
> }
Is it a problem that we convert a dablk to a db block on the way to a
byte conversion? i.e., that seems like a lossy conversion, assuming I am
correctly understanding that a dablk is equivalent to a filesystem block
and a db block is a directory block (1 or more dablks).
Perhaps this isn't a problem due to how this is used, but I just wanted
to call it out. The code uses this implementation irrespective of this
patch, and the patch looks Ok to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> /*
> diff --git a/fs/xfs/xfs_dir2.c b/fs/xfs/xfs_dir2.c
> index a7d3884..f76008c 100644
> --- a/fs/xfs/xfs_dir2.c
> +++ b/fs/xfs/xfs_dir2.c
> @@ -624,7 +624,7 @@ xfs_dir2_grow_inode(
> if (error)
> return error;
>
> - *dbp = xfs_dir2_da_to_db(mp, (xfs_dablk_t)bno);
> + *dbp = xfs_dir2_da_to_db(args->geo, (xfs_dablk_t)bno);
>
> /*
> * Update file's size if this is the data space and it grew.
> @@ -705,7 +705,7 @@ xfs_dir2_shrink_inode(
> dp = args->dp;
> mp = dp->i_mount;
> tp = args->trans;
> - da = xfs_dir2_db_to_da(mp, db);
> + da = xfs_dir2_db_to_da(args->geo, db);
> /*
> * Unmap the fsblock(s).
> */
> diff --git a/fs/xfs/xfs_dir2_data.c b/fs/xfs/xfs_dir2_data.c
> index bae8b5b..890c940 100644
> --- a/fs/xfs/xfs_dir2_data.c
> +++ b/fs/xfs/xfs_dir2_data.c
> @@ -584,8 +584,8 @@ xfs_dir3_data_init(
> /*
> * Get the buffer set up for the block.
> */
> - error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(mp, blkno), -1, &bp,
> - XFS_DATA_FORK);
> + error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, blkno),
> + -1, &bp, XFS_DATA_FORK);
> if (error)
> return error;
> bp->b_ops = &xfs_dir3_data_buf_ops;
> diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
> index dc4ef19..046ba4a 100644
> --- a/fs/xfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/xfs_dir2_leaf.c
> @@ -350,8 +350,8 @@ xfs_dir3_leaf_get_buf(
> ASSERT(bno >= xfs_dir2_byte_to_db(mp, XFS_DIR2_LEAF_OFFSET) &&
> bno < xfs_dir2_byte_to_db(mp, XFS_DIR2_FREE_OFFSET));
>
> - error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(mp, bno), -1, &bp,
> - XFS_DATA_FORK);
> + error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, bno),
> + -1, &bp, XFS_DATA_FORK);
> if (error)
> return error;
>
> @@ -403,7 +403,7 @@ xfs_dir2_block_to_leaf(
> if ((error = xfs_da_grow_inode(args, &blkno))) {
> return error;
> }
> - ldb = xfs_dir2_da_to_db(mp, blkno);
> + ldb = xfs_dir2_da_to_db(args->geo, blkno);
> ASSERT(ldb == xfs_dir2_byte_to_db(mp, XFS_DIR2_LEAF_OFFSET));
> /*
> * Initialize the leaf block, get a buffer for it.
> @@ -828,8 +828,8 @@ xfs_dir2_leaf_addname(
> * Just read that one in.
> */
> error = xfs_dir3_data_read(tp, dp,
> - xfs_dir2_db_to_da(mp, use_block),
> - -1, &dbp);
> + xfs_dir2_db_to_da(args->geo, use_block),
> + -1, &dbp);
> if (error) {
> xfs_trans_brelse(tp, lbp);
> return error;
> @@ -1269,8 +1269,8 @@ xfs_dir2_leaf_lookup_int(
> if (dbp)
> xfs_trans_brelse(tp, dbp);
> error = xfs_dir3_data_read(tp, dp,
> - xfs_dir2_db_to_da(mp, newdb),
> - -1, &dbp);
> + xfs_dir2_db_to_da(args->geo, newdb),
> + -1, &dbp);
> if (error) {
> xfs_trans_brelse(tp, lbp);
> return error;
> @@ -1310,8 +1310,8 @@ xfs_dir2_leaf_lookup_int(
> if (cidb != curdb) {
> xfs_trans_brelse(tp, dbp);
> error = xfs_dir3_data_read(tp, dp,
> - xfs_dir2_db_to_da(mp, cidb),
> - -1, &dbp);
> + xfs_dir2_db_to_da(args->geo, cidb),
> + -1, &dbp);
> if (error) {
> xfs_trans_brelse(tp, lbp);
> return error;
> @@ -1609,7 +1609,8 @@ xfs_dir2_leaf_trim_data(
> /*
> * Read the offending data block. We need its buffer.
> */
> - error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(mp, db), -1, &dbp);
> + error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(args->geo, db),
> + -1, &dbp);
> if (error)
> return error;
>
> diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
> index 1bcaaa0..3b70d56 100644
> --- a/fs/xfs/xfs_dir2_node.c
> +++ b/fs/xfs/xfs_dir2_node.c
> @@ -195,17 +195,18 @@ xfs_dir2_free_try_read(
>
> static int
> xfs_dir3_free_get_buf(
> - struct xfs_trans *tp,
> - struct xfs_inode *dp,
> + xfs_da_args_t *args,
> xfs_dir2_db_t fbno,
> struct xfs_buf **bpp)
> {
> + struct xfs_trans *tp = args->trans;
> + struct xfs_inode *dp = args->dp;
> struct xfs_mount *mp = dp->i_mount;
> struct xfs_buf *bp;
> int error;
> struct xfs_dir3_icfree_hdr hdr;
>
> - error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(mp, fbno),
> + error = xfs_da_get_buf(tp, dp, xfs_dir2_db_to_da(args->geo, fbno),
> -1, &bp, XFS_DATA_FORK);
> if (error)
> return error;
> @@ -319,7 +320,7 @@ xfs_dir2_leaf_to_node(
> /*
> * Get the buffer for the new freespace block.
> */
> - error = xfs_dir3_free_get_buf(tp, dp, fdb, &fbp);
> + error = xfs_dir3_free_get_buf(args, fdb, &fbp);
> if (error)
> return error;
>
> @@ -605,7 +606,8 @@ xfs_dir2_leafn_lookup_for_addname(
> xfs_trans_brelse(tp, curbp);
>
> error = xfs_dir2_free_read(tp, dp,
> - xfs_dir2_db_to_da(mp, newfdb),
> + xfs_dir2_db_to_da(args->geo,
> + newfdb),
> &curbp);
> if (error)
> return error;
> @@ -746,7 +748,8 @@ xfs_dir2_leafn_lookup_for_entry(
> curbp = state->extrablk.bp;
> } else {
> error = xfs_dir3_data_read(tp, dp,
> - xfs_dir2_db_to_da(mp, newdb),
> + xfs_dir2_db_to_da(args->geo,
> + newdb),
> -1, &curbp);
> if (error)
> return error;
> @@ -1246,7 +1249,8 @@ xfs_dir2_leafn_remove(
> * read in the free block.
> */
> fdb = dp->d_ops->db_to_fdb(mp, db);
> - error = xfs_dir2_free_read(tp, dp, xfs_dir2_db_to_da(mp, fdb),
> + error = xfs_dir2_free_read(tp, dp,
> + xfs_dir2_db_to_da(args->geo, fdb),
> &fbp);
> if (error)
> return error;
> @@ -1336,7 +1340,7 @@ xfs_dir2_leafn_split(
> /*
> * Initialize the new leaf block.
> */
> - error = xfs_dir3_leaf_get_buf(args, xfs_dir2_da_to_db(mp, blkno),
> + error = xfs_dir3_leaf_get_buf(args, xfs_dir2_da_to_db(args->geo, blkno),
> &newblk->bp, XFS_DIR2_LEAFN_MAGIC);
> if (error)
> return error;
> @@ -1729,7 +1733,7 @@ xfs_dir2_node_addname_int(
>
> if ((error = xfs_bmap_last_offset(dp, &fo, XFS_DATA_FORK)))
> return error;
> - lastfbno = xfs_dir2_da_to_db(mp, (xfs_dablk_t)fo);
> + lastfbno = xfs_dir2_da_to_db(args->geo, (xfs_dablk_t)fo);
> fbno = ifbno;
> }
> /*
> @@ -1766,8 +1770,8 @@ xfs_dir2_node_addname_int(
> * to avoid it.
> */
> error = xfs_dir2_free_try_read(tp, dp,
> - xfs_dir2_db_to_da(mp, fbno),
> - &fbp);
> + xfs_dir2_db_to_da(args->geo, fbno),
> + &fbp);
> if (error)
> return error;
> if (!fbp)
> @@ -1837,8 +1841,8 @@ xfs_dir2_node_addname_int(
> */
> fbno = dp->d_ops->db_to_fdb(mp, dbno);
> error = xfs_dir2_free_try_read(tp, dp,
> - xfs_dir2_db_to_da(mp, fbno),
> - &fbp);
> + xfs_dir2_db_to_da(args->geo, fbno),
> + &fbp);
> if (error)
> return error;
>
> @@ -1878,7 +1882,7 @@ xfs_dir2_node_addname_int(
> /*
> * Get a buffer for the new block.
> */
> - error = xfs_dir3_free_get_buf(tp, dp, fbno, &fbp);
> + error = xfs_dir3_free_get_buf(args, fbno, &fbp);
> if (error)
> return error;
> free = fbp->b_addr;
> @@ -1946,7 +1950,8 @@ xfs_dir2_node_addname_int(
> /*
> * Read the data block in.
> */
> - error = xfs_dir3_data_read(tp, dp, xfs_dir2_db_to_da(mp, dbno),
> + error = xfs_dir3_data_read(tp, dp,
> + xfs_dir2_db_to_da(args->geo, dbno),
> -1, &dbp);
> if (error)
> return error;
> @@ -2265,9 +2270,9 @@ xfs_dir2_node_trim_free(
> /*
> * Blow the block away.
> */
> - if ((error =
> - xfs_dir2_shrink_inode(args, xfs_dir2_da_to_db(mp, (xfs_dablk_t)fo),
> - bp))) {
> + error = xfs_dir2_shrink_inode(args,
> + xfs_dir2_da_to_db(args->geo, (xfs_dablk_t)fo), bp);
> + if (error) {
> /*
> * Can't fail with ENOSPC since that only happens with no
> * space reservation, when breaking up an extent into two
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index ec912c8..57e9247 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -396,14 +396,14 @@ xfs_dir2_leaf_readbuf(
> * No valid mappings, so no more data blocks.
> */
> if (!mip->map_valid) {
> - *curoff = xfs_dir2_da_to_byte(mp, mip->map_off);
> + *curoff = xfs_dir2_da_to_byte(mp->m_dir_geo, mip->map_off);
> goto out;
> }
>
> /*
> * Read the directory block starting at the first mapping.
> */
> - mip->curdb = xfs_dir2_da_to_db(mp, map->br_startoff);
> + mip->curdb = xfs_dir2_da_to_db(mp->m_dir_geo, map->br_startoff);
> error = xfs_dir3_data_read(NULL, dp, map->br_startoff,
> map->br_blockcount >= mp->m_dirblkfsbs ?
> XFS_FSB_TO_DADDR(mp, map->br_startblock) : -1, &bp);
> @@ -536,7 +536,7 @@ xfs_dir2_leaf_getdents(
> * Force this conversion through db so we truncate the offset
> * down to get the start of the data block.
> */
> - map_info->map_off = xfs_dir2_db_to_da(mp,
> + map_info->map_off = xfs_dir2_db_to_da(mp->m_dir_geo,
> xfs_dir2_byte_to_db(mp, curoff));
>
> /*
> --
> 1.9.0
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-05-23 19:06 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-23 0:03 [PATCH 00/16 V2] xfs: introduce struct xfs_da_geometry Dave Chinner
2014-05-23 0:03 ` [PATCH 01/16] xfs: introduce directory geometry structure Dave Chinner
2014-05-23 19:04 ` Brian Foster
2014-05-26 4:28 ` Dave Chinner
2014-05-26 13:29 ` Brian Foster
2014-05-26 21:39 ` Dave Chinner
2014-05-23 0:03 ` [PATCH 02/16] xfs: move directory block translatiosn to xfs_da_btree.h Dave Chinner
2014-05-23 19:05 ` Brian Foster
2014-05-27 10:46 ` Christoph Hellwig
2014-05-27 23:06 ` Dave Chinner
2014-05-28 5:28 ` Christoph Hellwig
2014-05-28 5:39 ` Dave Chinner
2014-05-23 0:03 ` [PATCH 03/16] xfs: kill XFS_DIR2...FIRSTDB macros Dave Chinner
2014-05-23 19:05 ` Brian Foster
2014-05-27 10:47 ` Christoph Hellwig
2014-05-23 0:03 ` [PATCH 04/16] xfs: convert dir byte/off conversion to xfs_da_geometry Dave Chinner
2014-05-23 19:05 ` Brian Foster
2014-05-23 0:03 ` [PATCH 05/16] xfs: convert directory dablk " Dave Chinner
2014-05-23 19:06 ` Brian Foster [this message]
2014-05-26 4:48 ` Dave Chinner
2014-05-23 0:03 ` [PATCH 06/16] xfs: convert directory db " Dave Chinner
2014-05-23 19:07 ` Brian Foster
2014-05-23 0:03 ` [PATCH 07/16] xfs: convert directory segment limits " Dave Chinner
2014-05-23 20:43 ` Brian Foster
2014-05-23 0:03 ` [PATCH 08/16] xfs: convert m_dirblkfsbs " Dave Chinner
2014-05-23 20:43 ` Brian Foster
2014-05-23 0:03 ` [PATCH 09/16] xfs: convert m_dirblksize " Dave Chinner
2014-05-27 15:59 ` Brian Foster
2014-05-23 0:03 ` [PATCH 10/16] xfs: convert dir/attr btree threshold " Dave Chinner
2014-05-27 15:59 ` Brian Foster
2014-05-23 0:03 ` [PATCH 11/16] xfs: move node entry counts " Dave Chinner
2014-05-27 15:59 ` Brian Foster
2014-05-27 23:47 ` Dave Chinner
2014-05-23 0:03 ` [PATCH 12/16] xfs: reduce direct usage of mp->m_dir_geo Dave Chinner
2014-05-27 15:59 ` Brian Foster
2014-05-27 23:53 ` Dave Chinner
2014-05-23 0:03 ` [PATCH 13/16] xfs: remove mp->m_dir_geo from directory logging Dave Chinner
2014-05-27 16:00 ` Brian Foster
2014-05-23 0:03 ` [PATCH 14/16] xfs: use xfs_da_geometry for block size in attr code Dave Chinner
2014-05-27 16:01 ` Brian Foster
2014-05-23 0:03 ` [PATCH 15/16] xfs: pass xfs_da_args to xfs_attr_leaf_newentsize Dave Chinner
2014-05-27 16:01 ` Brian Foster
2014-05-23 0:03 ` [PATCH 16/16] xfs: repalce attr LBSIZE with xfs_da_geometry Dave Chinner
2014-05-27 16:01 ` Brian Foster
-- strict thread matches above, loose matches on Subject: below --
2014-05-28 6:04 [PATCH 00/16 V3] xfs: introduce struct xfs_da_geometry Dave Chinner
2014-05-28 6:04 ` [PATCH 05/16] xfs: convert directory dablk conversion to xfs_da_geometry Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140523190622.GF8343@laptop.bfoster \
--to=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.