public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c
Date: Mon, 14 Sep 2015 15:18:17 -0400	[thread overview]
Message-ID: <20150914191816.GD34083@bfoster.bfoster> (raw)
In-Reply-To: <1441827251-13128-5-git-send-email-sandeen@sandeen.net>

On Wed, Sep 09, 2015 at 02:34:02PM -0500, Eric Sandeen wrote:
> verify_da_path and traverse_int_dablock are similar to
> verify_dir2_path and traverse_int_dir2block, but one
> difference is that the dir2 code reads using the
> multibuffer capable da_read_buf() routine, whereas
> the attr code doesn't need to, and just calls
> libxfs_readbuf.
> 
> The multibuffer code falls back just fine when the
> geometry indicates that it's not needed, so use that
> same code in the attribute routines, and remove
> another dir2 / da difference.  We make da_read_buf()
> non-static to facilitate this.
> 
> Finally, add a local *geo to these routines,
> to make the code even more similar at this point.
> The geometry will get passed in later in the series.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  repair/attr_repair.c |   49 +++++++++++++++++++++++++++++++++----------------
>  repair/dir2.c        |   22 +++++++++++++---------
>  repair/dir2.h        |    8 ++++++++
>  3 files changed, 54 insertions(+), 25 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index aba0782..fe81df4 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		xfs_dablk_t		*rbno,
>  		int			whichfork)
>  {
> +	bmap_ext_t		*bmp;

struct bmap_ext

(and several other places below)

>  	xfs_dablk_t		bno;
>  	int			i;
> +	int			nex;
>  	xfs_da_intnode_t	*node;
> +	bmap_ext_t		lbmp;
>  	xfs_fsblock_t		fsbno;
>  	xfs_buf_t		*bp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_attr_geo;
> +
>  	/*
>  	 * traverse down left-side of tree until we hit the
>  	 * left-most leaf block setting up the btree cursor along
> @@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		/*
>  		 * read in each block along the way and set up cursor
>  		 */
> -		fsbno = blkmap_get(da_cursor->blkmap, bno);
> +		nex = blkmap_getn(da_cursor->blkmap, bno,
> +				geo->fsbcount, &bmp, &lbmp);
>  

...
attr_repair.c: In function ‘process_attributes’:
attr_repair.c:185:5: warning: ‘fsbno’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     do_warn(
...

Brian

> -		if (fsbno == NULLFSBLOCK)
> +		if (nex == 0)
>  			goto error_out;
>  
> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> +		if (bmp != &lbmp)
> +			free(bmp);
> +
>  		if (!bp) {
>  			if (whichfork == XFS_DATA_FORK)
>  				do_warn(
> @@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> -		if (nodehdr.count > mp->m_attr_geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
> -				da_cursor->ino,
> -				nodehdr.count,
> -				mp->m_attr_geo->node_ents);
> +				da_cursor->ino, nodehdr.count, geo->node_ents);
>  			libxfs_putbuf(bp);
>  			goto error_out;
>  		}
> @@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t	*mp,
>  	int			bad;
>  	int			entry;
>  	int			this_level = p_level + 1;
> +	bmap_ext_t		*bmp;
> +	int			nex;
> +	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_attr_geo;
> +
>  	/*
>  	 * index is currently set to point to the entry that
>  	 * should be processed now in this level.
> @@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t	*mp,
>  		 */
>  		dabno = nodehdr.forw;
>  		ASSERT(dabno != 0);
> -		fsbno = blkmap_get(cursor->blkmap, dabno);
> -
> -		if (fsbno == NULLFSBLOCK) {
> -			do_warn(_("can't get map info for block %u "
> -				  "of directory inode %" PRIu64 "\n"),
> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
> +			&bmp, &lbmp);
> +		if (nex == 0) {
> +			do_warn(
> +_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			return(1);
>  		}
>  
> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> +		fsbno = bmp[0].startblock;
> +
> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> +		if (bmp != &lbmp)
> +			free(bmp);
> +
>  		if (!bp) {
>  			do_warn(
>  	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
> @@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t	*mp,
>  				dabno, fsbno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.count > mp->m_attr_geo->node_ents) {
> +		if (nodehdr.count > geo->node_ents) {
>  			do_warn(
>  	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>  				nodehdr.count,
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 54c49eb..44367c6 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -92,7 +92,7 @@ namecheck(char *name, int length)
>   * Multibuffer handling.
>   * V2 directory blocks can be noncontiguous, needing multiple buffers.
>   */
> -static struct xfs_buf *
> +struct xfs_buf *
>  da_read_buf(
>  	xfs_mount_t	*mp,
>  	int		nex,
> @@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>  	int			nex;
>  	xfs_da_intnode_t	*node;
>  	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_dir_geo;
> +
>  	/*
>  	 * traverse down left-side of tree until we hit the
>  	 * left-most leaf block setting up the btree cursor along
> @@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>  		 * read in each block along the way and set up cursor
>  		 */
>  		nex = blkmap_getn(da_cursor->blkmap, bno,
> -				mp->m_dir_geo->fsbcount, &bmp, &lbmp);
> +				geo->fsbcount, &bmp, &lbmp);
>  
>  		if (nex == 0)
>  			goto error_out;
> @@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>  			goto error_out;
>  		}
>  		btree = M_DIROPS(mp)->node_tree_p(node);
> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> -			libxfs_putbuf(bp);
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(
>  _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
> -				da_cursor->ino,
> -				nodehdr.count,
> -				mp->m_dir_geo->node_ents);
> +				da_cursor->ino, nodehdr.count, geo->node_ents);
> +			libxfs_putbuf(bp);
>  			goto error_out;
>  		}
>  		/*
> @@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t	*mp,
>  	bmap_ext_t		*bmp;
>  	int			nex;
>  	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_dir_geo;
> +
>  	/*
>  	 * index is currently set to point to the entry that
>  	 * should be processed now in this level.
> @@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
>  		 */
>  		dabno = nodehdr.forw;
>  		ASSERT(dabno != 0);
> -		nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>  			&bmp, &lbmp);
>  		if (nex == 0) {
>  			do_warn(
> @@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(
>  _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>  				nodehdr.count,
> diff --git a/repair/dir2.h b/repair/dir2.h
> index 3cc1941..186633f 100644
> --- a/repair/dir2.h
> +++ b/repair/dir2.h
> @@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor  {
>  	struct blkmap		*blkmap;
>  } dir2_bt_cursor_t;
>  
> +#include "bmap.h"	/* Goes away in later refactoring */
> +struct xfs_buf *
> +da_read_buf(
> +	xfs_mount_t	*mp,
> +	int		nex,
> +	bmap_ext_t	*bmp,
> +	const struct xfs_buf_ops *ops);
> +
>  int
>  process_dir2(
>  	xfs_mount_t	*mp,
> -- 
> 1.7.1
> 
> _______________________________________________
> 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

  reply	other threads:[~2015-09-14 19:18 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 19:33 [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c Eric Sandeen
2015-09-09 19:33 ` [PATCH 01/13] xfs_repair: remove trace-only 'n' member from da_level_state Eric Sandeen
2015-09-14 19:17   ` Brian Foster
2015-09-09 19:34 ` [PATCH 02/13] xfs_repair: remove type from da & dir2 cursors Eric Sandeen
2015-09-14 19:18   ` Brian Foster
2015-09-09 19:34 ` [PATCH 03/13] xfs_repair: make CRC checking consistent in path verification Eric Sandeen
2015-09-14 19:18   ` Brian Foster
2015-09-09 19:34 ` [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c Eric Sandeen
2015-09-14 19:18   ` Brian Foster [this message]
2015-09-14 19:24     ` Eric Sandeen
2015-09-14 19:30       ` Brian Foster
2015-09-09 19:34 ` [PATCH 05/13] xfs_repair: fix use-after-free in verify_final_dir2_path Eric Sandeen
2015-09-14 19:18   ` Brian Foster
2015-09-09 19:34 ` [PATCH 06/13] xfs_repair: add XR_DIR_TRACE to dir2.c Eric Sandeen
2015-09-14 19:18   ` Brian Foster
2015-09-09 19:34 ` [PATCH 07/13] xfs_repair: Remove BUF_PTR from attr_repair.c Eric Sandeen
2015-09-14 19:44   ` Brian Foster
2015-09-09 19:34 ` [PATCH 08/13] xfs_repair: catch bad level/depth in da node Eric Sandeen
2015-09-14 19:44   ` Brian Foster
2015-09-09 19:34 ` [PATCH 09/13] xfs_repair: better checking of v5 attributes Eric Sandeen
2015-09-14 19:44   ` Brian Foster
2015-09-23 17:53     ` Eric Sandeen
2015-09-09 19:34 ` [PATCH 10/13] xfs_repair: Remove more differences between attr & dir2 Eric Sandeen
2015-09-14 19:55   ` Brian Foster
2015-09-09 19:34 ` [PATCH 11/13] xfs_repair: whitespace & comments Eric Sandeen
2015-09-14 19:56   ` Brian Foster
2015-09-09 19:34 ` [PATCH 12/13] xfs_repair: move common dir2 and attr_repair code to da_util.c Eric Sandeen
2015-09-09 19:34 ` [PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c Eric Sandeen
2015-09-14 20:06   ` Brian Foster
2015-09-14 20:11     ` Eric Sandeen
2015-09-10  9:22 ` [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c Carlos Maiolino
2015-09-10 16:51   ` Eric Sandeen
2015-09-11  8:20     ` Carlos Maiolino

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=20150914191816.GD34083@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=sandeen@sandeen.net \
    --cc=xfs@oss.sgi.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox