diff options
| author | Bryn M. Reeves <bmr@redhat.com> | 2016-12-10 13:07:03 +0000 |
|---|---|---|
| committer | Bryn M. Reeves <bmr@redhat.com> | 2016-12-10 13:31:12 +0000 |
| commit | d8ba8ee9aea0b23a4ff03d19f7ddbe6511cdcbbd (patch) | |
| tree | 65c361b3d2d97e01bcfb5ad7184e9c3cac31040d | |
| parent | 1de3e106c96f6df7b32e072b6a1a3f924fa041bc (diff) | |
| download | lvm2-d8ba8ee9aea0b23a4ff03d19f7ddbe6511cdcbbd.tar.gz | |
libdm: use a private pool for filemap extent table
When mapping regions to a file descriptor, a temporary table of
extent descriptors is built using the dm_pool object building
interface.
Previously this use borrowed the dms->mem region and counter
table pool (since nothing can interleave with the allocation
while the caller is still in dm_stats_create_regions_from_fd()).
This turns out to be problematic for error recovery. When a
region creation operation fails partway through file mapping,
we need to roll back the set of already created regions and
this requires a listed handle: the dm_stats_list() will then
allocate from the same pool as the extents; we either have
to throw away valid list data, or leak the extent table, to
return the handle in a valid state.
Avoid this problem by creating a new, temporary mem pool in
_stats_create_file_regions() to hold the extent data, and
discarding it on exit from the function.
| -rw-r--r-- | WHATS_NEW_DM | 1 | ||||
| -rw-r--r-- | libdm/libdm-stats.c | 19 |
2 files changed, 17 insertions, 3 deletions
diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index 41138760e..73707b63c 100644 --- a/WHATS_NEW_DM +++ b/WHATS_NEW_DM @@ -1,5 +1,6 @@ Version 1.02.138 - ===================================== + Separate filemap extent allocation from region table. Fix segmentation fault when filemap region creation fails Fix performance of region cleanup for failed filemap creation. Fix very slow region deletion with many regions. diff --git a/libdm/libdm-stats.c b/libdm/libdm-stats.c index 34870e367..78b66ab40 100644 --- a/libdm/libdm-stats.c +++ b/libdm/libdm-stats.c @@ -4324,6 +4324,7 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd, int precise, uint64_t *count) { uint64_t *regions = NULL, i, max_region; + struct dm_pool *extent_mem = NULL; struct _extent *extents = NULL; char *hist_arg = NULL; struct statfs fsbuf; @@ -4357,9 +4358,19 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd, return 0; } - if (!(extents = _stats_get_extents_for_file(dms->mem, fd, count))) + /* Use a temporary, private pool for the extent table. This avoids + * hijacking the dms->mem (region table) pool which would lead to + * interleaving temporary allocations with dm_stats_list() data, + * causing complications in the error path. + */ + if (!(extent_mem = dm_pool_create("extents", sizeof(*extents)))) return_0; + if (!(extents = _stats_get_extents_for_file(extent_mem, fd, count))) { + dm_pool_destroy(extent_mem); + return_0; + } + if (bounds) { /* _build_histogram_arg enables precise if vals < 1ms. */ if (!(hist_arg = _build_histogram_arg(bounds, &precise))) @@ -4386,7 +4397,8 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd, if (bounds) dm_free(hist_arg); - dm_pool_free(dms->mem, extents); + dm_pool_free(extent_mem, extents); + dm_pool_destroy(extent_mem); return regions; out_remove: @@ -4407,7 +4419,8 @@ out_remove: *count = 0; out: - dm_pool_free(dms->mem, extents); + dm_pool_free(extent_mem, extents); + dm_pool_destroy(extent_mem); dm_free(hist_arg); dm_free(regions); return NULL; |