On Sat, Sep 29, 2012 at 04:49:03PM -0600, Anand Tiwari wrote: > Subject: [PATCH] xfs_repair: detect realtime extent shared by multiple > records in extent map > > XFS currently can have records in extent map, which starts from unaligned start block w.r.t rextsize. > xfs_repair considers this as a bug (multiple claims for a real-time extent) and deletes the file. > This patch addresses the issue, by comparing current and previous records and make sure they are > contiguous and not overlapped. > > Signed-off-by: Anand Tiwari <anand.tiwari@xxxxxxxxx> > --- > repair/dinode.c | 37 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/repair/dinode.c b/repair/dinode.c > index 5a2da39..5537f1c 100644 > --- a/repair/dinode.c > +++ b/repair/dinode.c > @@ -406,6 +406,7 @@ verify_agbno(xfs_mount_t *mp, > static int > process_rt_rec( > xfs_mount_t *mp, > + xfs_bmbt_irec_t *prev, > xfs_bmbt_irec_t *irec, > xfs_ino_t ino, > xfs_drfsbno_t *tot, > @@ -413,8 +414,11 @@ process_rt_rec( > { > xfs_dfsbno_t b; > xfs_drtbno_t ext; > + xfs_drtbno_t start_block; > + xfs_filblks_t block_count; > int state; > int pwe; /* partially-written extent */ > + int rtext_remainder; /* start block is not aligned w.r.t rextsize */ > > /* > * check numeric validity of the extent > @@ -461,12 +465,32 @@ _("malformed rt inode extent [%" PRIu64 " %" PRIu64 "] (fs rtext size = %u)\n"), > return 1; > } > > + /* If we have start of record unaligned w.r.t to rextsize, see > + * if we are sharing this realtime extent with previous record. sharing is only > + * allowed with previous extent. fail otherwise. > + * Also, we above condition is true, align start block and block count > + */ Comment format is: /* * this is a * multiline comment */ > + rtext_remainder = irec->br_startblock % mp->m_sb.sb_rextsize; > + if (rtext_remainder) { > + do_warn( > +_("data fork in rt ino %" PRIu64 " has unalinged start block %"PRIu64 "\n"), unaligned > + ino, > + irec->br_startblock); indent these a bit so the fact they are part of the do_warn function call is obvious. i.e. do_warn( <format string> ino, irec->br_startblock); > + if ((prev->br_startoff + prev->br_blockcount == irec->br_startoff) && > + (prev->br_startblock + prev->br_blockcount == irec->br_startblock)) { > + start_block = irec->br_startblock + (mp->m_sb.sb_rextsize - rtext_remainder); > + block_count = irec->br_blockcount - (mp->m_sb.sb_rextsize - rtext_remainder); > + } > + } else { > + start_block = irec->br_startblock; > + block_count = irec->br_blockcount; > + } So this just changes the loop below not to check the first part of the new extent and therefore avoid the duplicate extent usage detection? Any plans to correct the bmapbt extents so that repeated repair runs don't keep warning about the same problem? > + > /* > * set the appropriate number of extents > * this iterates block by block, this can be optimised using extents > */ > - for (b = irec->br_startblock; b < irec->br_startblock + > - irec->br_blockcount; b += mp->m_sb.sb_rextsize) { > + for (b = start_block; b < start_block + block_count; b += mp->m_sb.sb_rextsize) { > ext = (xfs_drtbno_t) b / mp->m_sb.sb_rextsize; > pwe = xfs_sb_version_hasextflgbit(&mp->m_sb) && > irec->br_state == XFS_EXT_UNWRITTEN && > @@ -548,6 +572,7 @@ process_bmbt_reclist_int( > int check_dups, > int whichfork) > { > + xfs_bmbt_irec_t prev; > xfs_bmbt_irec_t irec; > xfs_dfilblks_t cp = 0; /* prev count */ > xfs_dfsbno_t sp = 0; /* prev start */ > @@ -574,6 +599,11 @@ process_bmbt_reclist_int( > else > ftype = _("regular"); > > + prev.br_startoff = 0; > + prev.br_blockcount = 0; > + prev.br_startblock = 0; > + prev.br_state = 0; memset(&prev, 0, sizeof(prev)); Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs