* [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block
@ 2016-12-22 17:41 Eric Sandeen
2016-12-22 17:44 ` [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int Eric Sandeen
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Eric Sandeen @ 2016-12-22 17:41 UTC (permalink / raw)
To: linux-xfs
We have this pattern all over the kernel & userspace:
if (xfs_sb_version_hascrc(&mp->m_sb))
xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1,
agno, XFS_BTREE_CRC_BLOCKS);
else
xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1,
agno, 0);
The last flag (XFS_BTREE_CRC_BLOCKS) can be gleaned from
the features on the superblock via mp just as the if/else does,
so no need to pass that in from the caller. That's patch 1.
Then the difference is simply CRC vs not-CRC magic. This can also
be determined inside the called function by passing in a btree number,
and looking up the proper magic in the xfs_magics array.
patch2 makes xfs_btree_magic() more generic, taking a
crc flag & btnum, instead of just a cursor.
patch3 then makes use of xfs_btree_magic in the block init routine
to determine the proper magic.
With those changes, the if/else can go away, and we can simply call:
xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0);
and let the lower function sort out crc vs. no crc differences
by itself.
It's not a huge reduction in code, but it reduces a lot of the
boilerplate, particularly in growfs code. This may also make it
slightly easier to factor the growfs tree init code into a loop
(not sure about that, yet) and/or to move mkfs & growfs tree init
code into a libxfs/ call.
libxfs/xfs_bmap.c | 19 ++++---------------
libxfs/xfs_bmap_btree.c | 10 ++--------
libxfs/xfs_btree.c | 34 +++++++++++++++++++++-------------
libxfs/xfs_btree.h | 4 ++--
libxfs/xfs_types.h | 2 ++
xfs_fsops.c | 39 +++++++++------------------------------
6 files changed, 40 insertions(+), 68 deletions(-)
-Eric
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int 2016-12-22 17:41 [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block Eric Sandeen @ 2016-12-22 17:44 ` Eric Sandeen 2017-01-03 18:28 ` Brian Foster 2016-12-22 17:47 ` [PATCH 2/3] xfs: make xfs_btree_magic more generic Eric Sandeen ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2016-12-22 17:44 UTC (permalink / raw) To: Eric Sandeen, linux-xfs xfs_btree_init_block_int() can determine whether crcs are in effect without the passed-in XFS_BTREE_CRC_BLOCKS flag; the mp argument allows us to determine this from the superblock. Remove the flag from callers, and use xfs_sb_version_hascrc(&mp->m_sb) internally instead. This removes one difference between the if & else cases in the callers. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index c27344c..e645aa8 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -727,7 +727,7 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, XFS_BMAP_CRC_MAGIC, 1, 1, ip->i_ino, - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); + XFS_BTREE_LONG_PTRS); else xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, XFS_BMAP_MAGIC, 1, 1, ip->i_ino, @@ -804,7 +804,7 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block_int(mp, ablock, abp->b_bn, XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); + XFS_BTREE_LONG_PTRS); else xfs_btree_init_block_int(mp, ablock, abp->b_bn, XFS_BMAP_MAGIC, 0, 0, ip->i_ino, diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c index 8007d2b..f0293d4 100644 --- a/fs/xfs/libxfs/xfs_bmap_btree.c +++ b/fs/xfs/libxfs/xfs_bmap_btree.c @@ -74,7 +74,7 @@ if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); + XFS_BTREE_LONG_PTRS); else xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, XFS_BMAP_MAGIC, 0, 0, ip->i_ino, diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index 5c8e6f2..f49fc2f 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -1090,6 +1090,8 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) __u64 owner, unsigned int flags) { + int crc = xfs_sb_version_hascrc(&mp->m_sb); + buf->bb_magic = cpu_to_be32(magic); buf->bb_level = cpu_to_be16(level); buf->bb_numrecs = cpu_to_be16(numrecs); @@ -1097,7 +1099,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) if (flags & XFS_BTREE_LONG_PTRS) { buf->bb_u.l.bb_leftsib = cpu_to_be64(NULLFSBLOCK); buf->bb_u.l.bb_rightsib = cpu_to_be64(NULLFSBLOCK); - if (flags & XFS_BTREE_CRC_BLOCKS) { + if (crc) { buf->bb_u.l.bb_blkno = cpu_to_be64(blkno); buf->bb_u.l.bb_owner = cpu_to_be64(owner); uuid_copy(&buf->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid); @@ -1110,7 +1112,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) buf->bb_u.s.bb_leftsib = cpu_to_be32(NULLAGBLOCK); buf->bb_u.s.bb_rightsib = cpu_to_be32(NULLAGBLOCK); - if (flags & XFS_BTREE_CRC_BLOCKS) { + if (crc) { buf->bb_u.s.bb_blkno = cpu_to_be64(blkno); buf->bb_u.s.bb_owner = cpu_to_be32(__owner); uuid_copy(&buf->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid); diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 93d12fa..bd20051 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -354,7 +354,7 @@ if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, - agno, XFS_BTREE_CRC_BLOCKS); + agno, 0); else xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, agno, 0); @@ -383,7 +383,7 @@ if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block(mp, bp, XFS_ABTC_CRC_MAGIC, 0, 1, - agno, XFS_BTREE_CRC_BLOCKS); + agno, 0); else xfs_btree_init_block(mp, bp, XFS_ABTC_MAGIC, 0, 1, agno, 0); @@ -414,7 +414,7 @@ } xfs_btree_init_block(mp, bp, XFS_RMAP_CRC_MAGIC, 0, 0, - agno, XFS_BTREE_CRC_BLOCKS); + agno, 0); block = XFS_BUF_TO_BLOCK(bp); @@ -490,7 +490,7 @@ if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block(mp, bp, XFS_IBT_CRC_MAGIC, 0, 0, - agno, XFS_BTREE_CRC_BLOCKS); + agno, 0); else xfs_btree_init_block(mp, bp, XFS_IBT_MAGIC, 0, 0, agno, 0); @@ -515,8 +515,7 @@ if (xfs_sb_version_hascrc(&mp->m_sb)) xfs_btree_init_block(mp, bp, XFS_FIBT_CRC_MAGIC, - 0, 0, agno, - XFS_BTREE_CRC_BLOCKS); + 0, 0, agno, 0); else xfs_btree_init_block(mp, bp, XFS_FIBT_MAGIC, 0, 0, agno, 0); @@ -541,8 +540,7 @@ } xfs_btree_init_block(mp, bp, XFS_REFC_CRC_MAGIC, - 0, 0, agno, - XFS_BTREE_CRC_BLOCKS); + 0, 0, agno, 0); error = xfs_bwrite(bp); xfs_buf_relse(bp); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int 2016-12-22 17:44 ` [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int Eric Sandeen @ 2017-01-03 18:28 ` Brian Foster 0 siblings, 0 replies; 13+ messages in thread From: Brian Foster @ 2017-01-03 18:28 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs On Thu, Dec 22, 2016 at 11:44:59AM -0600, Eric Sandeen wrote: > xfs_btree_init_block_int() can determine whether crcs are > in effect without the passed-in XFS_BTREE_CRC_BLOCKS flag; > the mp argument allows us to determine this from the > superblock. Remove the flag from callers, and use > xfs_sb_version_hascrc(&mp->m_sb) internally instead. > > This removes one difference between the if & else cases > in the callers. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index c27344c..e645aa8 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -727,7 +727,7 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, > XFS_BMAP_CRC_MAGIC, 1, 1, ip->i_ino, > - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); > + XFS_BTREE_LONG_PTRS); > else > xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, > XFS_BMAP_MAGIC, 1, 1, ip->i_ino, > @@ -804,7 +804,7 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block_int(mp, ablock, abp->b_bn, > XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, > - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); > + XFS_BTREE_LONG_PTRS); > else > xfs_btree_init_block_int(mp, ablock, abp->b_bn, > XFS_BMAP_MAGIC, 0, 0, ip->i_ino, > diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c > index 8007d2b..f0293d4 100644 > --- a/fs/xfs/libxfs/xfs_bmap_btree.c > +++ b/fs/xfs/libxfs/xfs_bmap_btree.c > @@ -74,7 +74,7 @@ > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, > XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, > - XFS_BTREE_LONG_PTRS | XFS_BTREE_CRC_BLOCKS); > + XFS_BTREE_LONG_PTRS); > else > xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, > XFS_BMAP_MAGIC, 0, 0, ip->i_ino, > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index 5c8e6f2..f49fc2f 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -1090,6 +1090,8 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > __u64 owner, > unsigned int flags) > { > + int crc = xfs_sb_version_hascrc(&mp->m_sb); > + > buf->bb_magic = cpu_to_be32(magic); > buf->bb_level = cpu_to_be16(level); > buf->bb_numrecs = cpu_to_be16(numrecs); > @@ -1097,7 +1099,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > if (flags & XFS_BTREE_LONG_PTRS) { > buf->bb_u.l.bb_leftsib = cpu_to_be64(NULLFSBLOCK); > buf->bb_u.l.bb_rightsib = cpu_to_be64(NULLFSBLOCK); > - if (flags & XFS_BTREE_CRC_BLOCKS) { > + if (crc) { > buf->bb_u.l.bb_blkno = cpu_to_be64(blkno); > buf->bb_u.l.bb_owner = cpu_to_be64(owner); > uuid_copy(&buf->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid); > @@ -1110,7 +1112,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > > buf->bb_u.s.bb_leftsib = cpu_to_be32(NULLAGBLOCK); > buf->bb_u.s.bb_rightsib = cpu_to_be32(NULLAGBLOCK); > - if (flags & XFS_BTREE_CRC_BLOCKS) { > + if (crc) { > buf->bb_u.s.bb_blkno = cpu_to_be64(blkno); > buf->bb_u.s.bb_owner = cpu_to_be32(__owner); > uuid_copy(&buf->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid); > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index 93d12fa..bd20051 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -354,7 +354,7 @@ > > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, > - agno, XFS_BTREE_CRC_BLOCKS); > + agno, 0); > else > xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, > agno, 0); > @@ -383,7 +383,7 @@ > > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block(mp, bp, XFS_ABTC_CRC_MAGIC, 0, 1, > - agno, XFS_BTREE_CRC_BLOCKS); > + agno, 0); > else > xfs_btree_init_block(mp, bp, XFS_ABTC_MAGIC, 0, 1, > agno, 0); > @@ -414,7 +414,7 @@ > } > > xfs_btree_init_block(mp, bp, XFS_RMAP_CRC_MAGIC, 0, 0, > - agno, XFS_BTREE_CRC_BLOCKS); > + agno, 0); > block = XFS_BUF_TO_BLOCK(bp); > > > @@ -490,7 +490,7 @@ > > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block(mp, bp, XFS_IBT_CRC_MAGIC, 0, 0, > - agno, XFS_BTREE_CRC_BLOCKS); > + agno, 0); > else > xfs_btree_init_block(mp, bp, XFS_IBT_MAGIC, 0, 0, > agno, 0); > @@ -515,8 +515,7 @@ > > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block(mp, bp, XFS_FIBT_CRC_MAGIC, > - 0, 0, agno, > - XFS_BTREE_CRC_BLOCKS); > + 0, 0, agno, 0); > else > xfs_btree_init_block(mp, bp, XFS_FIBT_MAGIC, 0, > 0, agno, 0); > @@ -541,8 +540,7 @@ > } > > xfs_btree_init_block(mp, bp, XFS_REFC_CRC_MAGIC, > - 0, 0, agno, > - XFS_BTREE_CRC_BLOCKS); > + 0, 0, agno, 0); > > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] xfs: make xfs_btree_magic more generic 2016-12-22 17:41 [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block Eric Sandeen 2016-12-22 17:44 ` [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int Eric Sandeen @ 2016-12-22 17:47 ` Eric Sandeen 2016-12-22 19:22 ` Darrick J. Wong 2016-12-22 19:43 ` [PATCH 2/3 V2] " Eric Sandeen 2016-12-22 17:51 ` [PATCH 3/3] xfs: remove boilerplate around xfs_btree_init_block Eric Sandeen 2017-01-16 23:13 ` [PATCH 0/3] xfs: reduce " Eric Sandeen 3 siblings, 2 replies; 13+ messages in thread From: Eric Sandeen @ 2016-12-22 17:47 UTC (permalink / raw) To: Eric Sandeen, linux-xfs Right now the xfs_btree_magic() define takes only a cursor; change this to take crc and btnum args to make it more generically useful, and move to a header file. This will allow xfs_btree_init_block_int callers which don't have a cursor to make use of the xfs_magics array, which will happen in the next patch. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index f49fc2f..cdb952a 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -50,8 +50,6 @@ XFS_BMAP_CRC_MAGIC, XFS_IBT_CRC_MAGIC, XFS_FIBT_CRC_MAGIC, XFS_REFC_CRC_MAGIC } }; -#define xfs_btree_magic(cur) \ - xfs_magics[!!((cur)->bc_flags & XFS_BTREE_CRC_BLOCKS)][cur->bc_btnum] STATIC int /* error (0 or EFSCORRUPTED) */ xfs_btree_check_lblock( @@ -62,10 +60,13 @@ { int lblock_ok = 1; /* block passes checks */ struct xfs_mount *mp; /* file system mount point */ + xfs_btnum_t btnum = cur->bc_btnum; + int crc; mp = cur->bc_mp; + crc = xfs_sb_version_hascrc(&mp->m_sb); - if (xfs_sb_version_hascrc(&mp->m_sb)) { + if (crc) { lblock_ok = lblock_ok && uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid) && @@ -74,7 +75,7 @@ } lblock_ok = lblock_ok && - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && be16_to_cpu(block->bb_level) == level && be16_to_cpu(block->bb_numrecs) <= cur->bc_ops->get_maxrecs(cur, level) && @@ -110,13 +111,16 @@ struct xfs_agf *agf; /* ag. freespace structure */ xfs_agblock_t agflen; /* native ag. freespace length */ int sblock_ok = 1; /* block passes checks */ + xfs_btnum_t btnum = cur->bc_btnum; + int crc; mp = cur->bc_mp; + crc = xfs_sb_version_hascrc(&mp->m_sb); agbp = cur->bc_private.a.agbp; agf = XFS_BUF_TO_AGF(agbp); agflen = be32_to_cpu(agf->agf_length); - if (xfs_sb_version_hascrc(&mp->m_sb)) { + if (crc) { sblock_ok = sblock_ok && uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid) && @@ -125,7 +129,7 @@ } sblock_ok = sblock_ok && - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && be16_to_cpu(block->bb_level) == level && be16_to_cpu(block->bb_numrecs) <= cur->bc_ops->get_maxrecs(cur, level) && @@ -1142,7 +1146,9 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) int level, int numrecs) { - __u64 owner; + __u64 owner; + int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); + xfs_btnum_t btnum = cur->bc_btnum; /* * we can pull the owner from the cursor right now as the different @@ -1156,7 +1162,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) owner = cur->bc_private.a.agno; xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - xfs_btree_magic(cur), level, numrecs, + xfs_btree_magic(crc, btnum), level, numrecs, owner, cur->bc_flags); } diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h index 8d74870..8ddacf4 100644 --- a/fs/xfs/libxfs/xfs_types.h +++ b/fs/xfs/libxfs/xfs_types.h @@ -113,6 +113,8 @@ XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX } xfs_btnum_t; +#define xfs_btree_magic(crc, btnum) xfs_magics[crc][btnum] + struct xfs_name { const unsigned char *name; int len; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] xfs: make xfs_btree_magic more generic 2016-12-22 17:47 ` [PATCH 2/3] xfs: make xfs_btree_magic more generic Eric Sandeen @ 2016-12-22 19:22 ` Darrick J. Wong 2016-12-22 19:43 ` [PATCH 2/3 V2] " Eric Sandeen 1 sibling, 0 replies; 13+ messages in thread From: Darrick J. Wong @ 2016-12-22 19:22 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs On Thu, Dec 22, 2016 at 11:47:26AM -0600, Eric Sandeen wrote: > Right now the xfs_btree_magic() define takes only a cursor; > change this to take crc and btnum args to make it more generically > useful, and move to a header file. > > This will allow xfs_btree_init_block_int callers which don't > have a cursor to make use of the xfs_magics array, which will > happen in the next patch. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index f49fc2f..cdb952a 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -50,8 +50,6 @@ > XFS_BMAP_CRC_MAGIC, XFS_IBT_CRC_MAGIC, XFS_FIBT_CRC_MAGIC, > XFS_REFC_CRC_MAGIC } > }; > -#define xfs_btree_magic(cur) \ > - xfs_magics[!!((cur)->bc_flags & XFS_BTREE_CRC_BLOCKS)][cur->bc_btnum] > > STATIC int /* error (0 or EFSCORRUPTED) */ > xfs_btree_check_lblock( > @@ -62,10 +60,13 @@ > { > int lblock_ok = 1; /* block passes checks */ > struct xfs_mount *mp; /* file system mount point */ > + xfs_btnum_t btnum = cur->bc_btnum; > + int crc; > > mp = cur->bc_mp; > + crc = xfs_sb_version_hascrc(&mp->m_sb); > > - if (xfs_sb_version_hascrc(&mp->m_sb)) { > + if (crc) { > lblock_ok = lblock_ok && > uuid_equal(&block->bb_u.l.bb_uuid, > &mp->m_sb.sb_meta_uuid) && > @@ -74,7 +75,7 @@ > } > > lblock_ok = lblock_ok && > - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && > + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && > be16_to_cpu(block->bb_level) == level && > be16_to_cpu(block->bb_numrecs) <= > cur->bc_ops->get_maxrecs(cur, level) && > @@ -110,13 +111,16 @@ > struct xfs_agf *agf; /* ag. freespace structure */ > xfs_agblock_t agflen; /* native ag. freespace length */ > int sblock_ok = 1; /* block passes checks */ > + xfs_btnum_t btnum = cur->bc_btnum; > + int crc; > > mp = cur->bc_mp; > + crc = xfs_sb_version_hascrc(&mp->m_sb); > agbp = cur->bc_private.a.agbp; > agf = XFS_BUF_TO_AGF(agbp); > agflen = be32_to_cpu(agf->agf_length); > > - if (xfs_sb_version_hascrc(&mp->m_sb)) { > + if (crc) { > sblock_ok = sblock_ok && > uuid_equal(&block->bb_u.s.bb_uuid, > &mp->m_sb.sb_meta_uuid) && > @@ -125,7 +129,7 @@ > } > > sblock_ok = sblock_ok && > - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && > + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && > be16_to_cpu(block->bb_level) == level && > be16_to_cpu(block->bb_numrecs) <= > cur->bc_ops->get_maxrecs(cur, level) && > @@ -1142,7 +1146,9 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > int level, > int numrecs) > { > - __u64 owner; > + __u64 owner; > + int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); > + xfs_btnum_t btnum = cur->bc_btnum; > > /* > * we can pull the owner from the cursor right now as the different > @@ -1156,7 +1162,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > owner = cur->bc_private.a.agno; > > xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, > - xfs_btree_magic(cur), level, numrecs, > + xfs_btree_magic(crc, btnum), level, numrecs, > owner, cur->bc_flags); > } > > diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h > index 8d74870..8ddacf4 100644 > --- a/fs/xfs/libxfs/xfs_types.h > +++ b/fs/xfs/libxfs/xfs_types.h > @@ -113,6 +113,8 @@ > XFS_BTNUM_INOi, XFS_BTNUM_FINOi, XFS_BTNUM_REFCi, XFS_BTNUM_MAX > } xfs_btnum_t; > > +#define xfs_btree_magic(crc, btnum) xfs_magics[crc][btnum] TBH I was expecting this to be a static inline function instead of a macro that looks like a function. Maybe an assert to catch things like crc=0, btnum=XFS_BTNUM_RMAP that aren't supposed to exist. (I see that the third patch has one assert, but why not just leave it in the helper to catch any error case?) --D > + > struct xfs_name { > const unsigned char *name; > int len; > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3 V2] xfs: make xfs_btree_magic more generic 2016-12-22 17:47 ` [PATCH 2/3] xfs: make xfs_btree_magic more generic Eric Sandeen 2016-12-22 19:22 ` Darrick J. Wong @ 2016-12-22 19:43 ` Eric Sandeen 2017-01-03 18:28 ` Brian Foster 1 sibling, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2016-12-22 19:43 UTC (permalink / raw) To: Eric Sandeen, linux-xfs Right now the xfs_btree_magic() define takes only a cursor; change this to take crc and btnum args to make it more generically useful, and move to a function. This will allow xfs_btree_init_block_int callers which don't have a cursor to make use of the xfs_magics array, which will happen in the next patch. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: turn xfs_btree_magic into a function with a built-in ASSERT diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index f49fc2f..81866dd 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -50,8 +50,15 @@ XFS_BMAP_CRC_MAGIC, XFS_IBT_CRC_MAGIC, XFS_FIBT_CRC_MAGIC, XFS_REFC_CRC_MAGIC } }; -#define xfs_btree_magic(cur) \ - xfs_magics[!!((cur)->bc_flags & XFS_BTREE_CRC_BLOCKS)][cur->bc_btnum] + +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum) +{ + __uint32_t magic = xfs_magics[crc][btnum]; + + /* Ensure we asked for crc for crc-only magics. */ + ASSERT(magic != 0); + return magic; +} STATIC int /* error (0 or EFSCORRUPTED) */ xfs_btree_check_lblock( @@ -62,10 +69,13 @@ { int lblock_ok = 1; /* block passes checks */ struct xfs_mount *mp; /* file system mount point */ + xfs_btnum_t btnum = cur->bc_btnum; + int crc; mp = cur->bc_mp; + crc = xfs_sb_version_hascrc(&mp->m_sb); - if (xfs_sb_version_hascrc(&mp->m_sb)) { + if (crc) { lblock_ok = lblock_ok && uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid) && @@ -74,7 +84,7 @@ } lblock_ok = lblock_ok && - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && be16_to_cpu(block->bb_level) == level && be16_to_cpu(block->bb_numrecs) <= cur->bc_ops->get_maxrecs(cur, level) && @@ -110,13 +120,16 @@ struct xfs_agf *agf; /* ag. freespace structure */ xfs_agblock_t agflen; /* native ag. freespace length */ int sblock_ok = 1; /* block passes checks */ + xfs_btnum_t btnum = cur->bc_btnum; + int crc; mp = cur->bc_mp; + crc = xfs_sb_version_hascrc(&mp->m_sb); agbp = cur->bc_private.a.agbp; agf = XFS_BUF_TO_AGF(agbp); agflen = be32_to_cpu(agf->agf_length); - if (xfs_sb_version_hascrc(&mp->m_sb)) { + if (crc) { sblock_ok = sblock_ok && uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid) && @@ -125,7 +138,7 @@ } sblock_ok = sblock_ok && - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && be16_to_cpu(block->bb_level) == level && be16_to_cpu(block->bb_numrecs) <= cur->bc_ops->get_maxrecs(cur, level) && @@ -1142,7 +1155,9 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) int level, int numrecs) { - __u64 owner; + __u64 owner; + int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); + xfs_btnum_t btnum = cur->bc_btnum; /* * we can pull the owner from the cursor right now as the different @@ -1156,7 +1171,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) owner = cur->bc_private.a.agno; xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - xfs_btree_magic(cur), level, numrecs, + xfs_btree_magic(crc, btnum), level, numrecs, owner, cur->bc_flags); } diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h index c2b01d1..3aaced3 100644 --- a/fs/xfs/libxfs/xfs_btree.h +++ b/fs/xfs/libxfs/xfs_btree.h @@ -76,6 +76,8 @@ #define XFS_BTNUM_RMAP ((xfs_btnum_t)XFS_BTNUM_RMAPi) #define XFS_BTNUM_REFC ((xfs_btnum_t)XFS_BTNUM_REFCi) +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum); + /* * For logging record fields. */ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3 V2] xfs: make xfs_btree_magic more generic 2016-12-22 19:43 ` [PATCH 2/3 V2] " Eric Sandeen @ 2017-01-03 18:28 ` Brian Foster 2017-01-03 18:34 ` Eric Sandeen 0 siblings, 1 reply; 13+ messages in thread From: Brian Foster @ 2017-01-03 18:28 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs On Thu, Dec 22, 2016 at 01:43:55PM -0600, Eric Sandeen wrote: > Right now the xfs_btree_magic() define takes only a cursor; > change this to take crc and btnum args to make it more generically > useful, and move to a function. > > This will allow xfs_btree_init_block_int callers which don't > have a cursor to make use of the xfs_magics array, which will > happen in the next patch. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > V2: turn xfs_btree_magic into a function with a built-in ASSERT > > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index f49fc2f..81866dd 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -50,8 +50,15 @@ > XFS_BMAP_CRC_MAGIC, XFS_IBT_CRC_MAGIC, XFS_FIBT_CRC_MAGIC, > XFS_REFC_CRC_MAGIC } > }; > -#define xfs_btree_magic(cur) \ > - xfs_magics[!!((cur)->bc_flags & XFS_BTREE_CRC_BLOCKS)][cur->bc_btnum] > + > +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum) > +{ > + __uint32_t magic = xfs_magics[crc][btnum]; > + > + /* Ensure we asked for crc for crc-only magics. */ Purely a nit, but for whatever reason this sentence confused me. Perhaps just drop it, or say something like "ensure we asked for a valid magic?" Either way: Reviewed-by: Brian Foster <bfoster@redhat.com> > + ASSERT(magic != 0); > + return magic; > +} > > STATIC int /* error (0 or EFSCORRUPTED) */ > xfs_btree_check_lblock( > @@ -62,10 +69,13 @@ > { > int lblock_ok = 1; /* block passes checks */ > struct xfs_mount *mp; /* file system mount point */ > + xfs_btnum_t btnum = cur->bc_btnum; > + int crc; > > mp = cur->bc_mp; > + crc = xfs_sb_version_hascrc(&mp->m_sb); > > - if (xfs_sb_version_hascrc(&mp->m_sb)) { > + if (crc) { > lblock_ok = lblock_ok && > uuid_equal(&block->bb_u.l.bb_uuid, > &mp->m_sb.sb_meta_uuid) && > @@ -74,7 +84,7 @@ > } > > lblock_ok = lblock_ok && > - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && > + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && > be16_to_cpu(block->bb_level) == level && > be16_to_cpu(block->bb_numrecs) <= > cur->bc_ops->get_maxrecs(cur, level) && > @@ -110,13 +120,16 @@ > struct xfs_agf *agf; /* ag. freespace structure */ > xfs_agblock_t agflen; /* native ag. freespace length */ > int sblock_ok = 1; /* block passes checks */ > + xfs_btnum_t btnum = cur->bc_btnum; > + int crc; > > mp = cur->bc_mp; > + crc = xfs_sb_version_hascrc(&mp->m_sb); > agbp = cur->bc_private.a.agbp; > agf = XFS_BUF_TO_AGF(agbp); > agflen = be32_to_cpu(agf->agf_length); > > - if (xfs_sb_version_hascrc(&mp->m_sb)) { > + if (crc) { > sblock_ok = sblock_ok && > uuid_equal(&block->bb_u.s.bb_uuid, > &mp->m_sb.sb_meta_uuid) && > @@ -125,7 +138,7 @@ > } > > sblock_ok = sblock_ok && > - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && > + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && > be16_to_cpu(block->bb_level) == level && > be16_to_cpu(block->bb_numrecs) <= > cur->bc_ops->get_maxrecs(cur, level) && > @@ -1142,7 +1155,9 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > int level, > int numrecs) > { > - __u64 owner; > + __u64 owner; > + int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); > + xfs_btnum_t btnum = cur->bc_btnum; > > /* > * we can pull the owner from the cursor right now as the different > @@ -1156,7 +1171,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > owner = cur->bc_private.a.agno; > > xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, > - xfs_btree_magic(cur), level, numrecs, > + xfs_btree_magic(crc, btnum), level, numrecs, > owner, cur->bc_flags); > } > > diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h > index c2b01d1..3aaced3 100644 > --- a/fs/xfs/libxfs/xfs_btree.h > +++ b/fs/xfs/libxfs/xfs_btree.h > @@ -76,6 +76,8 @@ > #define XFS_BTNUM_RMAP ((xfs_btnum_t)XFS_BTNUM_RMAPi) > #define XFS_BTNUM_REFC ((xfs_btnum_t)XFS_BTNUM_REFCi) > > +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum); > + > /* > * For logging record fields. > */ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3 V2] xfs: make xfs_btree_magic more generic 2017-01-03 18:28 ` Brian Foster @ 2017-01-03 18:34 ` Eric Sandeen 0 siblings, 0 replies; 13+ messages in thread From: Eric Sandeen @ 2017-01-03 18:34 UTC (permalink / raw) To: Brian Foster; +Cc: Eric Sandeen, linux-xfs On 1/3/17 12:28 PM, Brian Foster wrote: > On Thu, Dec 22, 2016 at 01:43:55PM -0600, Eric Sandeen wrote: >> Right now the xfs_btree_magic() define takes only a cursor; >> change this to take crc and btnum args to make it more generically >> useful, and move to a function. >> >> This will allow xfs_btree_init_block_int callers which don't >> have a cursor to make use of the xfs_magics array, which will >> happen in the next patch. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> V2: turn xfs_btree_magic into a function with a built-in ASSERT >> >> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c >> index f49fc2f..81866dd 100644 >> --- a/fs/xfs/libxfs/xfs_btree.c >> +++ b/fs/xfs/libxfs/xfs_btree.c >> @@ -50,8 +50,15 @@ >> XFS_BMAP_CRC_MAGIC, XFS_IBT_CRC_MAGIC, XFS_FIBT_CRC_MAGIC, >> XFS_REFC_CRC_MAGIC } >> }; >> -#define xfs_btree_magic(cur) \ >> - xfs_magics[!!((cur)->bc_flags & XFS_BTREE_CRC_BLOCKS)][cur->bc_btnum] >> + >> +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum) >> +{ >> + __uint32_t magic = xfs_magics[crc][btnum]; >> + >> + /* Ensure we asked for crc for crc-only magics. */ > > Purely a nit, but for whatever reason this sentence confused me. > Perhaps just drop it, or say something like "ensure we asked for a valid > magic?" Hm, ok, it's supposed to mean "make sure CRC ==1 if we are asking for a magic number for a structure which exists only on filesystems w/ CRCs" If not, we'd get 0 for magic, and that's the assert. But yeah, I can see how the sentence isn't very clear. :( -Eric > Either way: > > Reviewed-by: Brian Foster <bfoster@redhat.com> > >> + ASSERT(magic != 0); >> + return magic; >> +} >> >> STATIC int /* error (0 or EFSCORRUPTED) */ >> xfs_btree_check_lblock( >> @@ -62,10 +69,13 @@ >> { >> int lblock_ok = 1; /* block passes checks */ >> struct xfs_mount *mp; /* file system mount point */ >> + xfs_btnum_t btnum = cur->bc_btnum; >> + int crc; >> >> mp = cur->bc_mp; >> + crc = xfs_sb_version_hascrc(&mp->m_sb); >> >> - if (xfs_sb_version_hascrc(&mp->m_sb)) { >> + if (crc) { >> lblock_ok = lblock_ok && >> uuid_equal(&block->bb_u.l.bb_uuid, >> &mp->m_sb.sb_meta_uuid) && >> @@ -74,7 +84,7 @@ >> } >> >> lblock_ok = lblock_ok && >> - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && >> + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && >> be16_to_cpu(block->bb_level) == level && >> be16_to_cpu(block->bb_numrecs) <= >> cur->bc_ops->get_maxrecs(cur, level) && >> @@ -110,13 +120,16 @@ >> struct xfs_agf *agf; /* ag. freespace structure */ >> xfs_agblock_t agflen; /* native ag. freespace length */ >> int sblock_ok = 1; /* block passes checks */ >> + xfs_btnum_t btnum = cur->bc_btnum; >> + int crc; >> >> mp = cur->bc_mp; >> + crc = xfs_sb_version_hascrc(&mp->m_sb); >> agbp = cur->bc_private.a.agbp; >> agf = XFS_BUF_TO_AGF(agbp); >> agflen = be32_to_cpu(agf->agf_length); >> >> - if (xfs_sb_version_hascrc(&mp->m_sb)) { >> + if (crc) { >> sblock_ok = sblock_ok && >> uuid_equal(&block->bb_u.s.bb_uuid, >> &mp->m_sb.sb_meta_uuid) && >> @@ -125,7 +138,7 @@ >> } >> >> sblock_ok = sblock_ok && >> - be32_to_cpu(block->bb_magic) == xfs_btree_magic(cur) && >> + be32_to_cpu(block->bb_magic) == xfs_btree_magic(crc, btnum) && >> be16_to_cpu(block->bb_level) == level && >> be16_to_cpu(block->bb_numrecs) <= >> cur->bc_ops->get_maxrecs(cur, level) && >> @@ -1142,7 +1155,9 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) >> int level, >> int numrecs) >> { >> - __u64 owner; >> + __u64 owner; >> + int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); >> + xfs_btnum_t btnum = cur->bc_btnum; >> >> /* >> * we can pull the owner from the cursor right now as the different >> @@ -1156,7 +1171,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) >> owner = cur->bc_private.a.agno; >> >> xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, >> - xfs_btree_magic(cur), level, numrecs, >> + xfs_btree_magic(crc, btnum), level, numrecs, >> owner, cur->bc_flags); >> } >> >> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h >> index c2b01d1..3aaced3 100644 >> --- a/fs/xfs/libxfs/xfs_btree.h >> +++ b/fs/xfs/libxfs/xfs_btree.h >> @@ -76,6 +76,8 @@ >> #define XFS_BTNUM_RMAP ((xfs_btnum_t)XFS_BTNUM_RMAPi) >> #define XFS_BTNUM_REFC ((xfs_btnum_t)XFS_BTNUM_REFCi) >> >> +__uint32_t xfs_btree_magic(int crc, xfs_btnum_t btnum); >> + >> /* >> * For logging record fields. >> */ >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] xfs: remove boilerplate around xfs_btree_init_block 2016-12-22 17:41 [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block Eric Sandeen 2016-12-22 17:44 ` [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int Eric Sandeen 2016-12-22 17:47 ` [PATCH 2/3] xfs: make xfs_btree_magic more generic Eric Sandeen @ 2016-12-22 17:51 ` Eric Sandeen 2016-12-22 19:44 ` [PATCH 3/3 V2] " Eric Sandeen 2017-01-16 23:13 ` [PATCH 0/3] xfs: reduce " Eric Sandeen 3 siblings, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2016-12-22 17:51 UTC (permalink / raw) To: Eric Sandeen, linux-xfs Now that xfs_btree_init_block_int is able to determine crc status from the passed-in mp, we can determine the proper magic as well if we are given a btree number, rather than an explicit magic value. Change xfs_btree_init_block[_int] callers to pass in the btree number, and let xfs_btree_init_block_int use the xfs_magics array via the xfs_btree_magic macro to determine which magic value is needed. This makes all of the if (crc) / else stanzas identical, and the if/else can be removed, leading to a single, common init_block call. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index e645aa8..b30ff0a 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -724,15 +724,9 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) * Fill in the root. */ block = ifp->if_broot; - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, - XFS_BMAP_CRC_MAGIC, 1, 1, ip->i_ino, + xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, + XFS_BTNUM_BMAP, 1, 1, ip->i_ino, XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, - XFS_BMAP_MAGIC, 1, 1, ip->i_ino, - XFS_BTREE_LONG_PTRS); - /* * Need a cursor. Can't allocate until bb_level is filled in. */ @@ -801,13 +795,8 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) */ abp->b_ops = &xfs_bmbt_buf_ops; ablock = XFS_BUF_TO_BLOCK(abp); - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, ablock, abp->b_bn, - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, ablock, abp->b_bn, - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, + xfs_btree_init_block_int(mp, ablock, abp->b_bn, + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, XFS_BTREE_LONG_PTRS); arp = XFS_BMBT_REC_ADDR(mp, ablock, 1); diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c index f0293d4..a8867a7 100644 --- a/fs/xfs/libxfs/xfs_bmap_btree.c +++ b/fs/xfs/libxfs/xfs_bmap_btree.c @@ -71,15 +71,9 @@ xfs_bmbt_key_t *tkp; __be64 *tpp; - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, + xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, XFS_BTREE_LONG_PTRS); - rblock->bb_level = dblock->bb_level; ASSERT(be16_to_cpu(rblock->bb_level) > 0); rblock->bb_numrecs = dblock->bb_numrecs; diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index cdb952a..8487005 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -1088,14 +1088,16 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) struct xfs_mount *mp, struct xfs_btree_block *buf, xfs_daddr_t blkno, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, unsigned int flags) { int crc = xfs_sb_version_hascrc(&mp->m_sb); + __u32 magic = xfs_btree_magic(crc, btnum); + ASSERT(magic != 0); buf->bb_magic = cpu_to_be32(magic); buf->bb_level = cpu_to_be16(level); buf->bb_numrecs = cpu_to_be16(numrecs); @@ -1129,14 +1131,14 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) xfs_btree_init_block( struct xfs_mount *mp, struct xfs_buf *bp, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, unsigned int flags) { xfs_btree_init_block_int(mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - magic, level, numrecs, owner, flags); + btnum, level, numrecs, owner, flags); } STATIC void @@ -1147,8 +1149,6 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) int numrecs) { __u64 owner; - int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); - xfs_btnum_t btnum = cur->bc_btnum; /* * we can pull the owner from the cursor right now as the different @@ -1162,7 +1162,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) owner = cur->bc_private.a.agno; xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - xfs_btree_magic(crc, btnum), level, numrecs, + cur->bc_btnum, level, numrecs, owner, cur->bc_flags); } diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h index c2b01d1..63de8dc 100644 --- a/fs/xfs/libxfs/xfs_btree.h +++ b/fs/xfs/libxfs/xfs_btree.h @@ -413,7 +413,7 @@ struct xfs_buf * /* buffer for agno/agbno */ xfs_btree_init_block( struct xfs_mount *mp, struct xfs_buf *bp, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, @@ -424,7 +424,7 @@ struct xfs_buf * /* buffer for agno/agbno */ struct xfs_mount *mp, struct xfs_btree_block *buf, xfs_daddr_t blkno, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index bd20051..e702928 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -352,12 +352,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0); arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); @@ -381,12 +376,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_ABTC_CRC_MAGIC, 0, 1, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_ABTC_MAGIC, 0, 1, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_CNT, 0, 1, agno, 0); arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); @@ -413,7 +403,7 @@ goto error0; } - xfs_btree_init_block(mp, bp, XFS_RMAP_CRC_MAGIC, 0, 0, + xfs_btree_init_block(mp, bp, XFS_BTNUM_RMAP, 0, 0, agno, 0); block = XFS_BUF_TO_BLOCK(bp); @@ -488,12 +478,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_IBT_CRC_MAGIC, 0, 0, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_IBT_MAGIC, 0, 0, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_INO , 0, 0, agno, 0); error = xfs_bwrite(bp); xfs_buf_relse(bp); @@ -513,12 +498,8 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_FIBT_CRC_MAGIC, + xfs_btree_init_block(mp, bp, XFS_BTNUM_FINO, 0, 0, agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_FIBT_MAGIC, 0, - 0, agno, 0); error = xfs_bwrite(bp); xfs_buf_relse(bp); @@ -539,7 +520,7 @@ goto error0; } - xfs_btree_init_block(mp, bp, XFS_REFC_CRC_MAGIC, + xfs_btree_init_block(mp, bp, XFS_BTNUM_REFC, 0, 0, agno, 0); error = xfs_bwrite(bp); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3 V2] xfs: remove boilerplate around xfs_btree_init_block 2016-12-22 17:51 ` [PATCH 3/3] xfs: remove boilerplate around xfs_btree_init_block Eric Sandeen @ 2016-12-22 19:44 ` Eric Sandeen 2017-01-03 18:28 ` Brian Foster 0 siblings, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2016-12-22 19:44 UTC (permalink / raw) To: Eric Sandeen, linux-xfs Now that xfs_btree_init_block_int is able to determine crc status from the passed-in mp, we can determine the proper magic as well if we are given a btree number, rather than an explicit magic value. Change xfs_btree_init_block[_int] callers to pass in the btree number, and let xfs_btree_init_block_int use the xfs_magics array via the xfs_btree_magic macro to determine which magic value is needed. This makes all of the if (crc) / else stanzas identical, and the if/else can be removed, leading to a single, common init_block call. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: remove ASSERT after xfs_btree_magic now that it's built into that function diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index e645aa8..b30ff0a 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -724,15 +724,9 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) * Fill in the root. */ block = ifp->if_broot; - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, - XFS_BMAP_CRC_MAGIC, 1, 1, ip->i_ino, + xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, + XFS_BTNUM_BMAP, 1, 1, ip->i_ino, XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, - XFS_BMAP_MAGIC, 1, 1, ip->i_ino, - XFS_BTREE_LONG_PTRS); - /* * Need a cursor. Can't allocate until bb_level is filled in. */ @@ -801,13 +795,8 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) */ abp->b_ops = &xfs_bmbt_buf_ops; ablock = XFS_BUF_TO_BLOCK(abp); - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, ablock, abp->b_bn, - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, ablock, abp->b_bn, - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, + xfs_btree_init_block_int(mp, ablock, abp->b_bn, + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, XFS_BTREE_LONG_PTRS); arp = XFS_BMBT_REC_ADDR(mp, ablock, 1); diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c index f0293d4..a8867a7 100644 --- a/fs/xfs/libxfs/xfs_bmap_btree.c +++ b/fs/xfs/libxfs/xfs_bmap_btree.c @@ -71,15 +71,9 @@ xfs_bmbt_key_t *tkp; __be64 *tpp; - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, - XFS_BTREE_LONG_PTRS); - else - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, + xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, XFS_BTREE_LONG_PTRS); - rblock->bb_level = dblock->bb_level; ASSERT(be16_to_cpu(rblock->bb_level) > 0); rblock->bb_numrecs = dblock->bb_numrecs; diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c index 81866dd..0dcc5f2 100644 --- a/fs/xfs/libxfs/xfs_btree.c +++ b/fs/xfs/libxfs/xfs_btree.c @@ -1097,13 +1097,14 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) struct xfs_mount *mp, struct xfs_btree_block *buf, xfs_daddr_t blkno, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, unsigned int flags) { int crc = xfs_sb_version_hascrc(&mp->m_sb); + __u32 magic = xfs_btree_magic(crc, btnum); buf->bb_magic = cpu_to_be32(magic); buf->bb_level = cpu_to_be16(level); @@ -1138,14 +1139,14 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) xfs_btree_init_block( struct xfs_mount *mp, struct xfs_buf *bp, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, unsigned int flags) { xfs_btree_init_block_int(mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - magic, level, numrecs, owner, flags); + btnum, level, numrecs, owner, flags); } STATIC void @@ -1156,8 +1157,6 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) int numrecs) { __u64 owner; - int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); - xfs_btnum_t btnum = cur->bc_btnum; /* * we can pull the owner from the cursor right now as the different @@ -1171,7 +1170,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) owner = cur->bc_private.a.agno; xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, - xfs_btree_magic(crc, btnum), level, numrecs, + cur->bc_btnum, level, numrecs, owner, cur->bc_flags); } diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h index 3aaced3..ae10b7b 100644 --- a/fs/xfs/libxfs/xfs_btree.h +++ b/fs/xfs/libxfs/xfs_btree.h @@ -415,7 +415,7 @@ struct xfs_buf * /* buffer for agno/agbno */ xfs_btree_init_block( struct xfs_mount *mp, struct xfs_buf *bp, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, @@ -426,7 +426,7 @@ struct xfs_buf * /* buffer for agno/agbno */ struct xfs_mount *mp, struct xfs_btree_block *buf, xfs_daddr_t blkno, - __u32 magic, + xfs_btnum_t btnum, __u16 level, __u16 numrecs, __u64 owner, diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index bd20051..e702928 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -352,12 +352,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0); arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); @@ -381,12 +376,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_ABTC_CRC_MAGIC, 0, 1, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_ABTC_MAGIC, 0, 1, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_CNT, 0, 1, agno, 0); arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); @@ -413,7 +403,7 @@ goto error0; } - xfs_btree_init_block(mp, bp, XFS_RMAP_CRC_MAGIC, 0, 0, + xfs_btree_init_block(mp, bp, XFS_BTNUM_RMAP, 0, 0, agno, 0); block = XFS_BUF_TO_BLOCK(bp); @@ -488,12 +478,7 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_IBT_CRC_MAGIC, 0, 0, - agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_IBT_MAGIC, 0, 0, - agno, 0); + xfs_btree_init_block(mp, bp, XFS_BTNUM_INO , 0, 0, agno, 0); error = xfs_bwrite(bp); xfs_buf_relse(bp); @@ -513,12 +498,8 @@ goto error0; } - if (xfs_sb_version_hascrc(&mp->m_sb)) - xfs_btree_init_block(mp, bp, XFS_FIBT_CRC_MAGIC, + xfs_btree_init_block(mp, bp, XFS_BTNUM_FINO, 0, 0, agno, 0); - else - xfs_btree_init_block(mp, bp, XFS_FIBT_MAGIC, 0, - 0, agno, 0); error = xfs_bwrite(bp); xfs_buf_relse(bp); @@ -539,7 +520,7 @@ goto error0; } - xfs_btree_init_block(mp, bp, XFS_REFC_CRC_MAGIC, + xfs_btree_init_block(mp, bp, XFS_BTNUM_REFC, 0, 0, agno, 0); error = xfs_bwrite(bp); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3 V2] xfs: remove boilerplate around xfs_btree_init_block 2016-12-22 19:44 ` [PATCH 3/3 V2] " Eric Sandeen @ 2017-01-03 18:28 ` Brian Foster 0 siblings, 0 replies; 13+ messages in thread From: Brian Foster @ 2017-01-03 18:28 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs On Thu, Dec 22, 2016 at 01:44:50PM -0600, Eric Sandeen wrote: > Now that xfs_btree_init_block_int is able to determine crc > status from the passed-in mp, we can determine the proper > magic as well if we are given a btree number, rather than > an explicit magic value. > > Change xfs_btree_init_block[_int] callers to pass in the > btree number, and let xfs_btree_init_block_int use the > xfs_magics array via the xfs_btree_magic macro to determine > which magic value is needed. This makes all of the > if (crc) / else stanzas identical, and the if/else can be > removed, leading to a single, common init_block call. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > > V2: remove ASSERT after xfs_btree_magic now that it's built > into that function > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index e645aa8..b30ff0a 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -724,15 +724,9 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) > * Fill in the root. > */ > block = ifp->if_broot; > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, > - XFS_BMAP_CRC_MAGIC, 1, 1, ip->i_ino, > + xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, > + XFS_BTNUM_BMAP, 1, 1, ip->i_ino, > XFS_BTREE_LONG_PTRS); > - else > - xfs_btree_init_block_int(mp, block, XFS_BUF_DADDR_NULL, > - XFS_BMAP_MAGIC, 1, 1, ip->i_ino, > - XFS_BTREE_LONG_PTRS); > - > /* > * Need a cursor. Can't allocate until bb_level is filled in. > */ > @@ -801,13 +795,8 @@ static inline bool xfs_bmap_wants_extents(struct xfs_inode *ip, int whichfork) > */ > abp->b_ops = &xfs_bmbt_buf_ops; > ablock = XFS_BUF_TO_BLOCK(abp); > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block_int(mp, ablock, abp->b_bn, > - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, > - XFS_BTREE_LONG_PTRS); > - else > - xfs_btree_init_block_int(mp, ablock, abp->b_bn, > - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, > + xfs_btree_init_block_int(mp, ablock, abp->b_bn, > + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, > XFS_BTREE_LONG_PTRS); > > arp = XFS_BMBT_REC_ADDR(mp, ablock, 1); > diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c > index f0293d4..a8867a7 100644 > --- a/fs/xfs/libxfs/xfs_bmap_btree.c > +++ b/fs/xfs/libxfs/xfs_bmap_btree.c > @@ -71,15 +71,9 @@ > xfs_bmbt_key_t *tkp; > __be64 *tpp; > > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, > - XFS_BMAP_CRC_MAGIC, 0, 0, ip->i_ino, > - XFS_BTREE_LONG_PTRS); > - else > - xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, > - XFS_BMAP_MAGIC, 0, 0, ip->i_ino, > + xfs_btree_init_block_int(mp, rblock, XFS_BUF_DADDR_NULL, > + XFS_BTNUM_BMAP, 0, 0, ip->i_ino, > XFS_BTREE_LONG_PTRS); > - > rblock->bb_level = dblock->bb_level; > ASSERT(be16_to_cpu(rblock->bb_level) > 0); > rblock->bb_numrecs = dblock->bb_numrecs; > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c > index 81866dd..0dcc5f2 100644 > --- a/fs/xfs/libxfs/xfs_btree.c > +++ b/fs/xfs/libxfs/xfs_btree.c > @@ -1097,13 +1097,14 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > struct xfs_mount *mp, > struct xfs_btree_block *buf, > xfs_daddr_t blkno, > - __u32 magic, > + xfs_btnum_t btnum, > __u16 level, > __u16 numrecs, > __u64 owner, > unsigned int flags) > { > int crc = xfs_sb_version_hascrc(&mp->m_sb); > + __u32 magic = xfs_btree_magic(crc, btnum); > > buf->bb_magic = cpu_to_be32(magic); > buf->bb_level = cpu_to_be16(level); > @@ -1138,14 +1139,14 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > xfs_btree_init_block( > struct xfs_mount *mp, > struct xfs_buf *bp, > - __u32 magic, > + xfs_btnum_t btnum, > __u16 level, > __u16 numrecs, > __u64 owner, > unsigned int flags) > { > xfs_btree_init_block_int(mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, > - magic, level, numrecs, owner, flags); > + btnum, level, numrecs, owner, flags); > } > > STATIC void > @@ -1156,8 +1157,6 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > int numrecs) > { > __u64 owner; > - int crc = xfs_sb_version_hascrc(&cur->bc_mp->m_sb); > - xfs_btnum_t btnum = cur->bc_btnum; > > /* > * we can pull the owner from the cursor right now as the different > @@ -1171,7 +1170,7 @@ static inline size_t xfs_btree_ptr_len(struct xfs_btree_cur *cur) > owner = cur->bc_private.a.agno; > > xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn, > - xfs_btree_magic(crc, btnum), level, numrecs, > + cur->bc_btnum, level, numrecs, > owner, cur->bc_flags); > } > > diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h > index 3aaced3..ae10b7b 100644 > --- a/fs/xfs/libxfs/xfs_btree.h > +++ b/fs/xfs/libxfs/xfs_btree.h > @@ -415,7 +415,7 @@ struct xfs_buf * /* buffer for agno/agbno */ > xfs_btree_init_block( > struct xfs_mount *mp, > struct xfs_buf *bp, > - __u32 magic, > + xfs_btnum_t btnum, > __u16 level, > __u16 numrecs, > __u64 owner, > @@ -426,7 +426,7 @@ struct xfs_buf * /* buffer for agno/agbno */ > struct xfs_mount *mp, > struct xfs_btree_block *buf, > xfs_daddr_t blkno, > - __u32 magic, > + xfs_btnum_t btnum, > __u16 level, > __u16 numrecs, > __u64 owner, > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index bd20051..e702928 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -352,12 +352,7 @@ > goto error0; > } > > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, > - agno, 0); > - else > - xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, > - agno, 0); > + xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0); > > arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); > arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); > @@ -381,12 +376,7 @@ > goto error0; > } > > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block(mp, bp, XFS_ABTC_CRC_MAGIC, 0, 1, > - agno, 0); > - else > - xfs_btree_init_block(mp, bp, XFS_ABTC_MAGIC, 0, 1, > - agno, 0); > + xfs_btree_init_block(mp, bp, XFS_BTNUM_CNT, 0, 1, agno, 0); > > arec = XFS_ALLOC_REC_ADDR(mp, XFS_BUF_TO_BLOCK(bp), 1); > arec->ar_startblock = cpu_to_be32(mp->m_ag_prealloc_blocks); > @@ -413,7 +403,7 @@ > goto error0; > } > > - xfs_btree_init_block(mp, bp, XFS_RMAP_CRC_MAGIC, 0, 0, > + xfs_btree_init_block(mp, bp, XFS_BTNUM_RMAP, 0, 0, > agno, 0); > block = XFS_BUF_TO_BLOCK(bp); > > @@ -488,12 +478,7 @@ > goto error0; > } > > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block(mp, bp, XFS_IBT_CRC_MAGIC, 0, 0, > - agno, 0); > - else > - xfs_btree_init_block(mp, bp, XFS_IBT_MAGIC, 0, 0, > - agno, 0); > + xfs_btree_init_block(mp, bp, XFS_BTNUM_INO , 0, 0, agno, 0); > > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > @@ -513,12 +498,8 @@ > goto error0; > } > > - if (xfs_sb_version_hascrc(&mp->m_sb)) > - xfs_btree_init_block(mp, bp, XFS_FIBT_CRC_MAGIC, > + xfs_btree_init_block(mp, bp, XFS_BTNUM_FINO, > 0, 0, agno, 0); > - else > - xfs_btree_init_block(mp, bp, XFS_FIBT_MAGIC, 0, > - 0, agno, 0); > > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > @@ -539,7 +520,7 @@ > goto error0; > } > > - xfs_btree_init_block(mp, bp, XFS_REFC_CRC_MAGIC, > + xfs_btree_init_block(mp, bp, XFS_BTNUM_REFC, > 0, 0, agno, 0); > > error = xfs_bwrite(bp); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block 2016-12-22 17:41 [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block Eric Sandeen ` (2 preceding siblings ...) 2016-12-22 17:51 ` [PATCH 3/3] xfs: remove boilerplate around xfs_btree_init_block Eric Sandeen @ 2017-01-16 23:13 ` Eric Sandeen 2017-01-17 1:14 ` Darrick J. Wong 3 siblings, 1 reply; 13+ messages in thread From: Eric Sandeen @ 2017-01-16 23:13 UTC (permalink / raw) To: Eric Sandeen, linux-xfs On 12/22/16 11:41 AM, Eric Sandeen wrote: > We have this pattern all over the kernel & userspace: > > if (xfs_sb_version_hascrc(&mp->m_sb)) > xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, > agno, XFS_BTREE_CRC_BLOCKS); > else > xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, > agno, 0); > > The last flag (XFS_BTREE_CRC_BLOCKS) can be gleaned from > the features on the superblock via mp just as the if/else does, > so no need to pass that in from the caller. That's patch 1. Ping on this series? Thanks, -Eric > Then the difference is simply CRC vs not-CRC magic. This can also > be determined inside the called function by passing in a btree number, > and looking up the proper magic in the xfs_magics array. > > patch2 makes xfs_btree_magic() more generic, taking a > crc flag & btnum, instead of just a cursor. > > patch3 then makes use of xfs_btree_magic in the block init routine > to determine the proper magic. > > With those changes, the if/else can go away, and we can simply call: > > xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0); > > and let the lower function sort out crc vs. no crc differences > by itself. > > It's not a huge reduction in code, but it reduces a lot of the > boilerplate, particularly in growfs code. This may also make it > slightly easier to factor the growfs tree init code into a loop > (not sure about that, yet) and/or to move mkfs & growfs tree init > code into a libxfs/ call. > > libxfs/xfs_bmap.c | 19 ++++--------------- > libxfs/xfs_bmap_btree.c | 10 ++-------- > libxfs/xfs_btree.c | 34 +++++++++++++++++++++------------- > libxfs/xfs_btree.h | 4 ++-- > libxfs/xfs_types.h | 2 ++ > xfs_fsops.c | 39 +++++++++------------------------------ > 6 files changed, 40 insertions(+), 68 deletions(-) > > -Eric > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block 2017-01-16 23:13 ` [PATCH 0/3] xfs: reduce " Eric Sandeen @ 2017-01-17 1:14 ` Darrick J. Wong 0 siblings, 0 replies; 13+ messages in thread From: Darrick J. Wong @ 2017-01-17 1:14 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eric Sandeen, linux-xfs On Mon, Jan 16, 2017 at 05:13:34PM -0600, Eric Sandeen wrote: > On 12/22/16 11:41 AM, Eric Sandeen wrote: > > We have this pattern all over the kernel & userspace: > > > > if (xfs_sb_version_hascrc(&mp->m_sb)) > > xfs_btree_init_block(mp, bp, XFS_ABTB_CRC_MAGIC, 0, 1, > > agno, XFS_BTREE_CRC_BLOCKS); > > else > > xfs_btree_init_block(mp, bp, XFS_ABTB_MAGIC, 0, 1, > > agno, 0); > > > > The last flag (XFS_BTREE_CRC_BLOCKS) can be gleaned from > > the features on the superblock via mp just as the if/else does, > > so no need to pass that in from the caller. That's patch 1. > > Ping on this series? Looks ok to me, it's on my list for 4.11. (Sorry, I've been a little slow since NYE, dealing with all the ice and snow eating my house.) --D > > Thanks, > -Eric > > > Then the difference is simply CRC vs not-CRC magic. This can also > > be determined inside the called function by passing in a btree number, > > and looking up the proper magic in the xfs_magics array. > > > > patch2 makes xfs_btree_magic() more generic, taking a > > crc flag & btnum, instead of just a cursor. > > > > patch3 then makes use of xfs_btree_magic in the block init routine > > to determine the proper magic. > > > > With those changes, the if/else can go away, and we can simply call: > > > > xfs_btree_init_block(mp, bp, XFS_BTNUM_BNO, 0, 1, agno, 0); > > > > and let the lower function sort out crc vs. no crc differences > > by itself. > > > > It's not a huge reduction in code, but it reduces a lot of the > > boilerplate, particularly in growfs code. This may also make it > > slightly easier to factor the growfs tree init code into a loop > > (not sure about that, yet) and/or to move mkfs & growfs tree init > > code into a libxfs/ call. > > > > libxfs/xfs_bmap.c | 19 ++++--------------- > > libxfs/xfs_bmap_btree.c | 10 ++-------- > > libxfs/xfs_btree.c | 34 +++++++++++++++++++++------------- > > libxfs/xfs_btree.h | 4 ++-- > > libxfs/xfs_types.h | 2 ++ > > xfs_fsops.c | 39 +++++++++------------------------------ > > 6 files changed, 40 insertions(+), 68 deletions(-) > > > > -Eric > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-01-17 1:14 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-22 17:41 [PATCH 0/3] xfs: reduce boilerplate around xfs_btree_init_block Eric Sandeen 2016-12-22 17:44 ` [PATCH 1/3] xfs: glean crc status from mp not flags in xfs_btree_init_block_int Eric Sandeen 2017-01-03 18:28 ` Brian Foster 2016-12-22 17:47 ` [PATCH 2/3] xfs: make xfs_btree_magic more generic Eric Sandeen 2016-12-22 19:22 ` Darrick J. Wong 2016-12-22 19:43 ` [PATCH 2/3 V2] " Eric Sandeen 2017-01-03 18:28 ` Brian Foster 2017-01-03 18:34 ` Eric Sandeen 2016-12-22 17:51 ` [PATCH 3/3] xfs: remove boilerplate around xfs_btree_init_block Eric Sandeen 2016-12-22 19:44 ` [PATCH 3/3 V2] " Eric Sandeen 2017-01-03 18:28 ` Brian Foster 2017-01-16 23:13 ` [PATCH 0/3] xfs: reduce " Eric Sandeen 2017-01-17 1:14 ` Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).