xfs
[Top] [All Lists]

[PATCH 4/5] repair: don't cache large blkmap allocations

To: xfs@xxxxxxxxxxx
Subject: [PATCH 4/5] repair: don't cache large blkmap allocations
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 10 Oct 2011 10:11:49 +1100
In-reply-to: <1318201910-11144-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1318201910-11144-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

We currently use thread local storage for storing blkmap allocations
from one inode to another as a way of reducing the number of short
term allocations we do. However, the stored allocations can only
ever grow, so once we've done a large allocation we never free than
memory even if we never need that much memory again. This can occur
if we have corrupted extent counts in inodes, and can greatly
increase the memory footprint of the repair process.

Hence if the cached blkmap array id greater than a reasonable number
of extents (say 100,000), then don't store the blkmap in TLS and
instead free it.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 repair/bmap.c   |   20 +++++++++++++++++++-
 repair/dinode.c |    6 ++----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/repair/bmap.c b/repair/bmap.c
index 3ee5eff..3e53457 100644
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -66,12 +66,30 @@ blkmap_alloc(
 
 /*
  * Free a block map.
+ *
+ * If the map is a large, uncommon size (say for hundreds of thousands of
+ * extents) then free it to release the memory. This prevents us from pinning
+ * large tracts of memory due to corrupted fork values or one-off fragmented
+ * files. Otherwise we have nothing to do but keep the memory around for the
+ * next inode
  */
 void
 blkmap_free(
        blkmap_t        *blkmap)
 {
-       /* nothing to do! - keep the memory around for the next inode */
+       if (!blkmap)
+               return;
+
+       /* consider more than 100k extents rare */
+       if (blkmap->naexts < 100 * 1024)
+               return;
+
+       if (blkmap == pthread_getspecific(dblkmap_key))
+               pthread_setspecific(dblkmap_key, NULL);
+       else
+               pthread_setspecific(ablkmap_key, NULL);
+
+       free(blkmap);
 }
 
 /*
diff --git a/repair/dinode.c b/repair/dinode.c
index 0cedc28..8ad4e94 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -2748,8 +2748,7 @@ process_dinode_int(xfs_mount_t *mp,
                break;
        }
 
-       if (dblkmap)
-               blkmap_free(dblkmap);
+       blkmap_free(dblkmap);
 
        /*
         * check nlinks feature, if it's a version 1 inode,
@@ -2768,8 +2767,7 @@ clear_bad_out:
 bad_out:
        *used = is_free;
        *isa_dir = 0;
-       if (dblkmap)
-               blkmap_free(dblkmap);
+       blkmap_free(dblkmap);
        return 1;
 }
 
-- 
1.7.5.4

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