From: Eric Sandeen <sandeen@sandeen.net>
To: Brian Foster <bfoster@redhat.com>
Cc: Eric Sandeen <sandeen@redhat.com>, linux-xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH 2/3 V2] xfs: make xfs_btree_magic more generic
Date: Tue, 3 Jan 2017 12:34:56 -0600 [thread overview]
Message-ID: <8205ee7f-790f-e5f1-916e-7c4c15a0a818@sandeen.net> (raw)
In-Reply-To: <20170103182823.GC18120@bfoster.bfoster>
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
>
next prev parent reply other threads:[~2017-01-03 18:44 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
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=8205ee7f-790f-e5f1-916e-7c4c15a0a818@sandeen.net \
--to=sandeen@sandeen.net \
--cc=bfoster@redhat.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@redhat.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.