]> err.no Git - linux-2.6/commitdiff
[XFS] fix extent corruption in xfs_iext_irec_compact_full()
authorLachlan McIlroy <lachlan@sgi.com>
Mon, 23 Jun 2008 03:25:02 +0000 (13:25 +1000)
committerNiv Sardi <xaiki@debian.org>
Mon, 28 Jul 2008 06:58:56 +0000 (16:58 +1000)
This function is used to compact the indirect extent list by moving
extents from one page to the previous to fill them up. After we move some
extents to an earlier page we need to shuffle the remaining extents to the
start of the page. The actual bug here is the second argument to memmove()
needs to index past the extents, that were copied to the previous page,
and move the remaining extents. For pages that are already full (ie
ext_avail == 0) the compaction code has no net effect so don't do it.

SGI-PV: 983337

SGI-Modid: xfs-linux-melb:xfs-kern:31332a

Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
Signed-off-by: Christoph Hellwig <hch@infradead.org>
fs/xfs/xfs_inode.c

index fcb1dcc6f0365ab92390275213cba4aa3de3b7d9..bedc66163176484b5dc7e7911637ab0059e6c0a8 100644 (file)
@@ -4532,39 +4532,63 @@ xfs_iext_irec_compact_full(
        int             nlists;                 /* number of irec's (ex lists) */
 
        ASSERT(ifp->if_flags & XFS_IFEXTIREC);
+
        nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
        erp = ifp->if_u1.if_ext_irec;
        ep = &erp->er_extbuf[erp->er_extcount];
        erp_next = erp + 1;
        ep_next = erp_next->er_extbuf;
+
        while (erp_idx < nlists - 1) {
+               /*
+                * Check how many extent records are available in this irec.
+                * If there is none skip the whole exercise.
+                */
                ext_avail = XFS_LINEAR_EXTS - erp->er_extcount;
-               ext_diff = MIN(ext_avail, erp_next->er_extcount);
-               memcpy(ep, ep_next, ext_diff * sizeof(xfs_bmbt_rec_t));
-               erp->er_extcount += ext_diff;
-               erp_next->er_extcount -= ext_diff;
-               /* Remove next page */
-               if (erp_next->er_extcount == 0) {
+               if (ext_avail) {
+
                        /*
-                        * Free page before removing extent record
-                        * so er_extoffs don't get modified in
-                        * xfs_iext_irec_remove.
+                        * Copy over as many as possible extent records into
+                        * the previous page.
                         */
-                       kmem_free(erp_next->er_extbuf);
-                       erp_next->er_extbuf = NULL;
-                       xfs_iext_irec_remove(ifp, erp_idx + 1);
-                       erp = &ifp->if_u1.if_ext_irec[erp_idx];
-                       nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-               /* Update next page */
-               } else {
-                       /* Move rest of page up to become next new page */
-                       memmove(erp_next->er_extbuf, ep_next,
-                               erp_next->er_extcount * sizeof(xfs_bmbt_rec_t));
-                       ep_next = erp_next->er_extbuf;
-                       memset(&ep_next[erp_next->er_extcount], 0,
-                               (XFS_LINEAR_EXTS - erp_next->er_extcount) *
-                               sizeof(xfs_bmbt_rec_t));
+                       ext_diff = MIN(ext_avail, erp_next->er_extcount);
+                       memcpy(ep, ep_next, ext_diff * sizeof(xfs_bmbt_rec_t));
+                       erp->er_extcount += ext_diff;
+                       erp_next->er_extcount -= ext_diff;
+
+                       /*
+                        * If the next irec is empty now we can simply
+                        * remove it.
+                        */
+                       if (erp_next->er_extcount == 0) {
+                               /*
+                                * Free page before removing extent record
+                                * so er_extoffs don't get modified in
+                                * xfs_iext_irec_remove.
+                                */
+                               kmem_free(erp_next->er_extbuf);
+                               erp_next->er_extbuf = NULL;
+                               xfs_iext_irec_remove(ifp, erp_idx + 1);
+                               erp = &ifp->if_u1.if_ext_irec[erp_idx];
+                               nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
+
+                       /*
+                        * If the next irec is not empty move up the content
+                        * that has not been copied to the previous page to
+                        * the beggining of this one.
+                        */
+                       } else {
+                               memmove(erp_next->er_extbuf, &ep_next[ext_diff],
+                                       erp_next->er_extcount *
+                                       sizeof(xfs_bmbt_rec_t));
+                               ep_next = erp_next->er_extbuf;
+                               memset(&ep_next[erp_next->er_extcount], 0,
+                                       (XFS_LINEAR_EXTS -
+                                               erp_next->er_extcount) *
+                                       sizeof(xfs_bmbt_rec_t));
+                       }
                }
+
                if (erp->er_extcount == XFS_LINEAR_EXTS) {
                        erp_idx++;
                        if (erp_idx < nlists)