On 28 Jul 2021 at 04:44, Darrick J. Wong wrote:
> On Mon, Jul 26, 2021 at 05:17:17PM +0530, Chandan Babu R wrote:
>> This commit replaces the macro XFS_DFORK_NEXTENTS() with the helper function
>> xfs_dfork_nextents(). As of this commit, xfs_dfork_nextents() returns the same
>> value as XFS_DFORK_NEXTENTS(). A future commit which extends inode's extent
>> counter fields will add more logic to this helper.
>>
>> This commit also replaces direct accesses to xfs_dinode->di_[a]nextents
>> with calls to xfs_dfork_nextents().
>>
>> No functional changes have been made.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
>> ---
>> db/bmap.c | 6 ++--
>> db/btdump.c | 4 +--
>> db/check.c | 27 ++++++++++-------
>> db/frag.c | 6 ++--
>> db/inode.c | 14 +++++----
>> db/metadump.c | 4 +--
>> libxfs/xfs_format.h | 4 ---
>> libxfs/xfs_inode_buf.c | 43 +++++++++++++++++++++++----
>> libxfs/xfs_inode_buf.h | 2 ++
>> libxfs/xfs_inode_fork.c | 11 ++++---
>> repair/attr_repair.c | 2 +-
>> repair/bmap_repair.c | 4 +--
>> repair/dinode.c | 66 ++++++++++++++++++++++++-----------------
>> repair/prefetch.c | 2 +-
>> 14 files changed, 123 insertions(+), 72 deletions(-)
>>
>> diff --git a/db/bmap.c b/db/bmap.c
>> index 50f0474bc..5e1ab9258 100644
>> --- a/db/bmap.c
>> +++ b/db/bmap.c
>> @@ -68,7 +68,7 @@ bmap(
>> ASSERT(fmt == XFS_DINODE_FMT_LOCAL || fmt == XFS_DINODE_FMT_EXTENTS ||
>> fmt == XFS_DINODE_FMT_BTREE);
>> if (fmt == XFS_DINODE_FMT_EXTENTS) {
>> - nextents = XFS_DFORK_NEXTENTS(dip, whichfork);
>> + nextents = xfs_dfork_nextents(mp, dip, whichfork);
>> xp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork);
>> for (ep = xp; ep < &xp[nextents] && n < nex; ep++) {
>> if (!bmap_one_extent(ep, &curoffset, eoffset, &n, bep))
>> @@ -158,9 +158,9 @@ bmap_f(
>> push_cur();
>> set_cur_inode(iocur_top->ino);
>> dip = iocur_top->data;
>> - if (be32_to_cpu(dip->di_nextents))
>> + if (xfs_dfork_nextents(mp, dip, XFS_DATA_FORK))
>> dfork = 1;
>> - if (be16_to_cpu(dip->di_anextents))
>> + if (xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK))
>> afork = 1;
>> pop_cur();
>> }
>> diff --git a/db/btdump.c b/db/btdump.c
>> index 920f595b4..59609fd2d 100644
>> --- a/db/btdump.c
>> +++ b/db/btdump.c
>> @@ -166,13 +166,13 @@ dump_inode(
>>
>> dip = iocur_top->data;
>> if (attrfork) {
>> - if (!dip->di_anextents ||
>> + if (!xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK) ||
>> dip->di_aformat != XFS_DINODE_FMT_BTREE) {
>> dbprintf(_("attr fork not in btree format\n"));
>> return 0;
>> }
>> } else {
>> - if (!dip->di_nextents ||
>> + if (!xfs_dfork_nextents(mp, dip, XFS_DATA_FORK) ||
>> dip->di_format != XFS_DINODE_FMT_BTREE) {
>> dbprintf(_("data fork not in btree format\n"));
>> return 0;
>> diff --git a/db/check.c b/db/check.c
>> index 0d923e3ae..fe422e0ca 100644
>> --- a/db/check.c
>> +++ b/db/check.c
>> @@ -2720,7 +2720,7 @@ process_exinode(
>> xfs_bmbt_rec_t *rp;
>>
>> rp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork);
>> - *nex = XFS_DFORK_NEXTENTS(dip, whichfork);
>> + *nex = xfs_dfork_nextents(mp, dip, whichfork);
>> if (*nex < 0 || *nex > XFS_DFORK_SIZE(dip, mp, whichfork) /
>> sizeof(xfs_bmbt_rec_t)) {
>> if (!sflag || id->ilist)
>> @@ -2744,12 +2744,14 @@ process_inode(
>> inodata_t *id = NULL;
>> xfs_ino_t ino;
>> xfs_extnum_t nextents = 0;
>> + xfs_extnum_t dnextents;
>> int security;
>> xfs_rfsblock_t totblocks;
>> xfs_rfsblock_t totdblocks = 0;
>> xfs_rfsblock_t totiblocks = 0;
>> dbm_t type;
>> xfs_extnum_t anextents = 0;
>> + xfs_extnum_t danextents;
>> xfs_rfsblock_t atotdblocks = 0;
>> xfs_rfsblock_t atotiblocks = 0;
>> xfs_qcnt_t bc = 0;
>> @@ -2878,14 +2880,17 @@ process_inode(
>> error++;
>> return;
>> }
>> +
>> + dnextents = xfs_dfork_nextents(mp, dip, XFS_DATA_FORK);
>> + danextents = xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK);
>> +
>> if (verbose || (id && id->ilist) || CHECK_BLIST(bno))
>> dbprintf(_("inode %lld mode %#o fmt %s "
>> "afmt %s "
>> "nex %d anex %d nblk %lld sz %lld%s%s%s%s%s%s%s\n"),
>> id->ino, mode, fmtnames[(int)dip->di_format],
>> fmtnames[(int)dip->di_aformat],
>> - be32_to_cpu(dip->di_nextents),
>> - be16_to_cpu(dip->di_anextents),
>> + dnextents, danextents,
>> be64_to_cpu(dip->di_nblocks), be64_to_cpu(dip->di_size),
>> diflags & XFS_DIFLAG_REALTIME ? " rt" : "",
>> diflags & XFS_DIFLAG_PREALLOC ? " pre" : "",
>> @@ -2903,19 +2908,19 @@ process_inode(
>> if (xfs_sb_version_hasmetadir(&mp->m_sb) &&
>> id->ino == mp->m_sb.sb_metadirino)
>> addlink_inode(id);
>> - blkmap = blkmap_alloc(be32_to_cpu(dip->di_nextents));
>> + blkmap = blkmap_alloc(dnextents);
>> break;
>> case S_IFREG:
>> if (diflags & XFS_DIFLAG_REALTIME)
>> type = DBM_RTDATA;
>> else if (id->ino == mp->m_sb.sb_rbmino) {
>> type = DBM_RTBITMAP;
>> - blkmap = blkmap_alloc(be32_to_cpu(dip->di_nextents));
>> + blkmap = blkmap_alloc(dnextents);
>> if (!xfs_sb_version_hasmetadir(&mp->m_sb))
>> addlink_inode(id);
>> } else if (id->ino == mp->m_sb.sb_rsumino) {
>> type = DBM_RTSUM;
>> - blkmap = blkmap_alloc(be32_to_cpu(dip->di_nextents));
>> + blkmap = blkmap_alloc(dnextents);
>> if (!xfs_sb_version_hasmetadir(&mp->m_sb))
>> addlink_inode(id);
>> }
>> @@ -2923,7 +2928,7 @@ process_inode(
>> id->ino == mp->m_sb.sb_gquotino ||
>> id->ino == mp->m_sb.sb_pquotino) {
>> type = DBM_QUOTA;
>> - blkmap = blkmap_alloc(be32_to_cpu(dip->di_nextents));
>> + blkmap = blkmap_alloc(dnextents);
>> if (!xfs_sb_version_hasmetadir(&mp->m_sb))
>> addlink_inode(id);
>> }
>> @@ -3006,17 +3011,17 @@ process_inode(
>> be64_to_cpu(dip->di_nblocks), id->ino, totblocks);
>> error++;
>> }
>> - if (nextents != be32_to_cpu(dip->di_nextents)) {
>> + if (nextents != dnextents) {
>> if (v)
>> dbprintf(_("bad nextents %d for inode %lld, counted %d\n"),
>> - be32_to_cpu(dip->di_nextents), id->ino, nextents);
>> + dnextents, id->ino, nextents);
>> error++;
>> }
>> - if (anextents != be16_to_cpu(dip->di_anextents)) {
>> + if (anextents != danextents) {
>> if (v)
>> dbprintf(_("bad anextents %d for inode %lld, counted "
>> "%d\n"),
>> - be16_to_cpu(dip->di_anextents), id->ino, anextents);
>> + danextents, id->ino, anextents);
>> error++;
>> }
>> if (type == DBM_DIR)
>> diff --git a/db/frag.c b/db/frag.c
>> index 90fa2131c..3e43a9a21 100644
>> --- a/db/frag.c
>> +++ b/db/frag.c
>> @@ -262,9 +262,11 @@ process_exinode(
>> int whichfork)
>> {
>> xfs_bmbt_rec_t *rp;
>> + xfs_extnum_t nextents;
>>
>> rp = (xfs_bmbt_rec_t *)XFS_DFORK_PTR(dip, whichfork);
>> - process_bmbt_reclist(rp, XFS_DFORK_NEXTENTS(dip, whichfork), extmapp);
>> + nextents = xfs_dfork_nextents(mp, dip, whichfork);
>> + process_bmbt_reclist(rp, nextents, extmapp);
>> }
>>
>> static void
>> @@ -275,7 +277,7 @@ process_fork(
>> extmap_t *extmap;
>> xfs_extnum_t nex;
>>
>> - nex = XFS_DFORK_NEXTENTS(dip, whichfork);
>> + nex = xfs_dfork_nextents(mp, dip, whichfork);
>> if (!nex)
>> return;
>> extmap = extmap_alloc(nex);
>> diff --git a/db/inode.c b/db/inode.c
>> index e59cff451..681f4f98a 100644
>> --- a/db/inode.c
>> +++ b/db/inode.c
>> @@ -278,7 +278,7 @@ inode_a_bmx_count(
>> return 0;
>> ASSERT((char *)XFS_DFORK_APTR(dip) - (char *)dip == byteize(startoff));
>> return dip->di_aformat == XFS_DINODE_FMT_EXTENTS ?
>> - be16_to_cpu(dip->di_anextents) : 0;
>> + xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK) : 0;
>> }
>>
>> static int
>> @@ -332,6 +332,7 @@ inode_a_size(
>> {
>> struct xfs_attr_shortform *asf;
>> xfs_dinode_t *dip;
>> + xfs_extnum_t nextents;
>>
>> ASSERT(startoff == 0);
>> ASSERT(idx == 0);
>> @@ -341,8 +342,8 @@ inode_a_size(
>> asf = (struct xfs_attr_shortform *)XFS_DFORK_APTR(dip);
>> return bitize(be16_to_cpu(asf->hdr.totsize));
>> case XFS_DINODE_FMT_EXTENTS:
>> - return (int)be16_to_cpu(dip->di_anextents) *
>> - bitsz(xfs_bmbt_rec_t);
>> + nextents = xfs_dfork_nextents(mp, dip, XFS_ATTR_FORK);
>> + return (int)nextents * bitsz(xfs_bmbt_rec_t);
>
> I think you can drop the cast (and convert the typedef) her.
>
>> case XFS_DINODE_FMT_BTREE:
>> return bitize((int)XFS_DFORK_ASIZE(dip, mp));
>> default:
>> @@ -503,7 +504,7 @@ inode_u_bmx_count(
>> dip = obj;
>> ASSERT((char *)XFS_DFORK_DPTR(dip) - (char *)dip == byteize(startoff));
>> return dip->di_format == XFS_DINODE_FMT_EXTENTS ?
>> - be32_to_cpu(dip->di_nextents) : 0;
>> + xfs_dfork_nextents(mp, dip, XFS_DATA_FORK) : 0;
>> }
>>
>> static int
>> @@ -589,6 +590,7 @@ inode_u_size(
>> int idx)
>> {
>> xfs_dinode_t *dip;
>> + xfs_extnum_t nextents;
>>
>> ASSERT(startoff == 0);
>> ASSERT(idx == 0);
>> @@ -599,8 +601,8 @@ inode_u_size(
>> case XFS_DINODE_FMT_LOCAL:
>> return bitize((int)be64_to_cpu(dip->di_size));
>> case XFS_DINODE_FMT_EXTENTS:
>> - return (int)be32_to_cpu(dip->di_nextents) *
>> - bitsz(xfs_bmbt_rec_t);
>> + nextents = xfs_dfork_nextents(mp, dip, XFS_DATA_FORK);
>> + return (int)nextents * bitsz(xfs_bmbt_rec_t);
>
> ...and here.
>
Ok. I will apply the two review comments before posting the next version.
> The rest of the db/repair changes look good.
>
> --D
--
chandan
