Re: [PATCH 4/5] xfs_db: don't error out when blocksize > 64 * inodesize

Brian Foster
2016-02-01T15:03:42Z

On Fri, Jan 22, 2016 at 04:35:25PM -0800, Darrick J. Wong wrote:
> When the block size is large enough that multiple inode chunks can fit
> in a single block (e.g. 64k blocks, 512 byte inodes) we must calculate
> the per-chunk block size and the buffer offset correctly so that check
> actually examines the correct metadata.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> db/check.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
>
> diff --git a/db/check.c b/db/check.c
> index 838db53..4e2768e 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -4386,6 +4386,7 @@ scanfunc_ino(
> __u16 holemask;
> xfs_agino_t rino;
> xfs_extlen_t cblocks;
> + int bufoff;
>
> if (be32_to_cpu(block->bb_magic) != XFS_IBT_MAGIC &&
> be32_to_cpu(block->bb_magic) != XFS_IBT_CRC_MAGIC) {
> @@ -4455,6 +4456,8 @@ scanfunc_ino(
> rino = agino + startidx;
> cblocks = (endidx - startidx) >>
> mp->m_sb.sb_inopblog;
> + if (cblocks == 0)
> + cblocks = 1;

A comment would be nice here (i.e., "handle the case where the entire
chunk is in a single block").

>
> /* Check the sparse chunk alignment */
> if (sparse &&
> @@ -4468,8 +4471,9 @@ scanfunc_ino(
> }
>
> /* Check the block map */
> - set_dbmap(seqno, XFS_AGINO_TO_AGBNO(mp, rino),
> - cblocks, DBM_INODE, seqno, bno);
> + if (XFS_AGINO_TO_OFFSET(mp, rino) == 0)
> + set_dbmap(seqno, XFS_AGINO_TO_AGBNO(mp, rino),
> + cblocks, DBM_INODE, seqno, bno);
>
> push_cur();
> set_cur(&typtab[TYP_INODE],
> @@ -4489,14 +4493,15 @@ scanfunc_ino(
> }
>
> /* Examine each inode in this chunk */
> - for (j = startidx; j < endidx; j++) {
> + bufoff = ((XFS_AGINO_TO_OFFSET(mp, rino) - startidx) << mp->m_sb.sb_inodelog);
> + for (j = startidx; j < endidx; j++, bufoff += (1 << mp->m_sb.sb_inodelog)) {

Here as well.

Though, does this work for sparse inodes? Suppose we start halfway into
a sparse inode record. So XFS_AGINO_TO_OFFSET(mp, rino) is 0 since we're
at the first inode of the fs block; and say startidx is 32 as that could
be the first real inode index in the record. It looks like we could go
off the rails in that case..?

Brian

> if (ino_issparse(&rp[i], j))
> continue;
> isfree = XFS_INOBT_IS_FREE_DISK(&rp[i], j);
> if (isfree)
> nfree++;
> process_inode(agf, agino + j,
> - (xfs_dinode_t *)((char *)iocur_top->data + ((j - startidx) << mp->m_sb.sb_inodelog)),
> + (xfs_dinode_t *)((char *)iocur_top->data + bufoff),
> isfree);
> }
> pop_cur();
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs