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

Dave Chinner
2011-10-10T01:09:07Z

From: Dave Chinner <dchinner@redhat.com>

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>
---
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 e290515..5fb27bc 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 39a0cb1..fb5e53a 100644
--- 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;
}

--
1.7.5.4