xfs
[Top] [All Lists]

Re: xfs_repair deleting realtime files.

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: xfs_repair deleting realtime files.
From: Anand Tiwari <tiwarikanand@xxxxxxxxx>
Date: Sat, 29 Sep 2012 16:49:03 -0600 (MDT)
Cc: Anand Tiwari <tiwarikanand@xxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:x-x-sender:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; bh=TycQ4A1XVUHTzhEOxE/Zu+LWsNK40tC2Ra7iToJIAAo=; b=yvlDB7XMeW7loSr4viBBRXkxEcbUZvFgSAwRJ5WM9SkQ49OXMbIP8kAUj8tyuVgRpG 2a315pdgyDzfXnhEP02koF+AQUgXGJaJtmb3uhOlx8modKyBz9Jv1l5nH/2WxqUasPZu AXwMGW/WR0oSOfzvy9wBAd1S2WcEREQX3JceqPmwRrFib8ae9aY+SYpgTY3XzCU74Q7F CFseJteiGyZzaA68PDstOudllFhHVvv5BvRom4P/j3Sz8Fq9UIpt8vabSi2IwdoLw34b 0cTwvk677gxuXCYYYPyps78vo5678NWJvpgYQv12tYbdEfVbsp/yfHLLL/ANWWqI/ZPV WUow==
In-reply-to: <20120928064742.GE25626@dastard>
References: <CAHt31_9K_vrzoqwSVsz-6VNVmMUzMyGCFEZfviRV-xPcUqv8-w@xxxxxxxxxxxxxx> <505BF45D.5050909@xxxxxxxxxxx> <20120924075551.GF20960@dastard> <CAHt31_8rEc93vpnbbKngY4uS0kAct3Z5A+2G0LmBzv5rWKdSfA@xxxxxxxxxxxxxx> <CAHt31__s2TNhXPa9JfDLdWPqr60Te9VDPKb4ieORF8JAL07YmQ@xxxxxxxxxxxxxx> <20120926024403.GH29154@dastard> <CAHt31__FPLsDdVCVwwLO0Sh9v_KK2pzU577nywFPAmf_C-gRcQ@xxxxxxxxxxxxxx> <20120926061732.GI29154@dastard> <CAHt31__59_XJw9EF7MniiYH5sS6CX44q9gGwxiqxx43k5OFZ4A@xxxxxxxxxxxxxx> <20120928064742.GE25626@dastard>
User-agent: Alpine 2.02 (DEB 1266 2009-07-14)

On Fri, 28 Sep 2012, Dave Chinner wrote:

> On Thu, Sep 27, 2012 at 07:27:16PM -0600, Anand Tiwari wrote:
> > On Wed, Sep 26, 2012 at 12:17 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> >
> > > On Tue, Sep 25, 2012 at 09:45:07PM -0600, Anand Tiwari wrote:
> > > > thanks Dave for prompt reply, I meant to implement option 2 as you
> > > > explained (option 3).  I will start working on it tomorrow. In the mean
> > > > time, I also had  to put something in xfs_repair for the files which
> > > > already exists on the disk. Would you guys willing to review/comment on
> > > > that ?
> > >
> > > Sure.
> > >
> > > Cheers,
> > >
> > > Dave.
> > > --
> > > Dave Chinner
> > > david@xxxxxxxxxxxxx
> > >
> >
> > following are my changes for xfs_repair. my goal  is to keep changes
> > minimum as they may not included in upstream. I had to do these changes as
> > we already have files with extent map not properly aligned.
> > As we know, this happens only when we are growing a file in a realtime
> > volume. by keeping this in mind, I am checking if start of a record in
> > extent map is not aligned, check previous record and if they are
> > contiguous, we can skip that part of record.
> >
> > let me know if you any issues with this or if someone has better approach.
> > I would like to use pointers for prev and irec but again, I wanted to keep
> > changes minimum
> >
> > thanks
> > anand
> >
> >
>
> Your mailer has removed all the whitespace from the patch. The files
> Documentation/SubmittingPatches and Documentation/email-clients.txt
> for help with how to send patches sanely via email. ;)
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>

okay,here is my second attempt, hope this time it work.

>From 77f8dc66fa5ce6b004f95dbb794322be35d4190a Mon Sep 17 00:00:00 2001
From: Anand Tiwari <anand.tiwari@xxxxxxxxx>
Date: Fri, 28 Sep 2012 15:45:55 -0600
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
+        */
+       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"),
+               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;
+       }
+
        /*
         * 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;
+
        for (i = 0; i < *numrecs; i++) {
                libxfs_bmbt_disk_get_all(rp + i, &irec);
                if (i == 0)
@@ -610,12 +640,13 @@ _("zero length extent (off = %" PRIu64 ", fsbno = %" 
PRIu64 ") in ino %" PRIu64
                         * realtime bitmaps don't use AG locks, so returning
                         * immediately is fine for this code path.
                         */
-                       if (process_rt_rec(mp, &irec, ino, tot, check_dups))
+                       if (process_rt_rec(mp, &prev, &irec, ino, tot, 
check_dups))
                                return 1;
                        /*
                         * skip rest of loop processing since 
that'irec.br_startblock
                         * all for regular file forks and attr forks
                         */
+                       memcpy(&prev, &irec, sizeof(xfs_bmbt_irec_t));
                        continue;
                }

-- 
1.7.9.5

<Prev in Thread] Current Thread [Next in Thread>