On Mon, Sep 14, 2015 at 02:24:53PM -0500, Eric Sandeen wrote:
>
>
> On 9/14/15 2:18 PM, Brian Foster wrote:
> > 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@xxxxxxxxxx>
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> >> ---
> >> 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;
>
> Well, yeah - I thought about that. The goal was to make it as close
> as possible to the other code, so I decided against the struct foo's.
>
> I could do a "get rid of typedefs" as the last patch to the new util
> code if you like.
>
Ah, right. Don't worry about this then.
> > 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]
>
> hm, whoops. Well, it goes away in the long run, but I can still fix
> that up.
>
I figured it might (haven't got that far yet), but I'd still rather not
have regressions in the history if we can help it, even if transient.
Brian
> Thanks,
> -Eric
>
> > 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@xxxxxxxxxxx
> >> http://oss.sgi.com/mailman/listinfo/xfs
> >
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
