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

From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 4/5] xfs_db: don't error out when blocksize > 64 * inodesize
Date: Mon, 1 Feb 2016 10:18:49 -0500	[thread overview]
Message-ID: <20160201151849.GC3992@bfoster.bfoster> (raw)
In-Reply-To: <20160123003525.2475.44264.stgit@birch.djwong.org>

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs