Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=d8ba8ee9aea0b23a4... Commit: d8ba8ee9aea0b23a4ff03d19f7ddbe6511cdcbbd Parent: 1de3e106c96f6df7b32e072b6a1a3f924fa041bc Author: Bryn M. Reeves bmr@redhat.com AuthorDate: Sat Dec 10 13:07:03 2016 +0000 Committer: Bryn M. Reeves bmr@redhat.com CommitterDate: Sat Dec 10 13:31:12 2016 +0000

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. --- WHATS_NEW_DM | 1 + libdm/libdm-stats.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index 4113876..73707b6 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 34870e3..78b66ab 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;