repair: don't cache large blkmap allocations

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@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>

diff --git a/repair/bmap.c b/repair/bmap.c
index e290515692cbceceb025a98962d9df8815885c44..5fb27bcac82ce25a09e417a4ca1f1e5fec859757 100644 (file)
--- 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 39a0cb1eb73f983a1168cea0d8bda26bed3ef4ac..fb5e53aa7b3d77438c06c2af35a34f2151da7ce3 100644 (file)
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -2807,8 +2807,7 @@ _("bad non-zero extent size %u for non-realtime/extsize inode %" PRIu64 ", "),
                break;
        }
 
-       if (dblkmap)
-               blkmap_free(dblkmap);
+       blkmap_free(dblkmap);
 
        /*
         * check nlinks feature, if it's a version 1 inode,
@@ -2827,8 +2826,7 @@ clear_bad_out:
 bad_out:
        *used = is_free;
        *isa_dir = 0;
-       if (dblkmap)
-               blkmap_free(dblkmap);
+       blkmap_free(dblkmap);
        return 1;
 }