1
0
mirror of git://sourceware.org/git/lvm2.git synced 2024-12-21 13:34:40 +03:00

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.
This commit is contained in:
Bryn M. Reeves 2016-12-10 13:07:03 +00:00
parent 1de3e106c9
commit d8ba8ee9ae
2 changed files with 17 additions and 3 deletions

View File

@ -1,5 +1,6 @@
Version 1.02.138 - Version 1.02.138 -
===================================== =====================================
Separate filemap extent allocation from region table.
Fix segmentation fault when filemap region creation fails Fix segmentation fault when filemap region creation fails
Fix performance of region cleanup for failed filemap creation. Fix performance of region cleanup for failed filemap creation.
Fix very slow region deletion with many regions. Fix very slow region deletion with many regions.

View File

@ -4324,6 +4324,7 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd,
int precise, uint64_t *count) int precise, uint64_t *count)
{ {
uint64_t *regions = NULL, i, max_region; uint64_t *regions = NULL, i, max_region;
struct dm_pool *extent_mem = NULL;
struct _extent *extents = NULL; struct _extent *extents = NULL;
char *hist_arg = NULL; char *hist_arg = NULL;
struct statfs fsbuf; struct statfs fsbuf;
@ -4357,9 +4358,19 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd,
return 0; 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; return_0;
if (!(extents = _stats_get_extents_for_file(extent_mem, fd, count))) {
dm_pool_destroy(extent_mem);
return_0;
}
if (bounds) { if (bounds) {
/* _build_histogram_arg enables precise if vals < 1ms. */ /* _build_histogram_arg enables precise if vals < 1ms. */
if (!(hist_arg = _build_histogram_arg(bounds, &precise))) 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) if (bounds)
dm_free(hist_arg); dm_free(hist_arg);
dm_pool_free(dms->mem, extents); dm_pool_free(extent_mem, extents);
dm_pool_destroy(extent_mem);
return regions; return regions;
out_remove: out_remove:
@ -4407,7 +4419,8 @@ out_remove:
*count = 0; *count = 0;
out: out:
dm_pool_free(dms->mem, extents); dm_pool_free(extent_mem, extents);
dm_pool_destroy(extent_mem);
dm_free(hist_arg); dm_free(hist_arg);
dm_free(regions); dm_free(regions);
return NULL; return NULL;