From 4aed5101a6a862e790b1c83330e9d45712019b4b Mon Sep 17 00:00:00 2001 From: Tomasz Gromadzki Date: Tue, 24 Oct 2023 08:03:13 +0200 Subject: [PATCH] common: decrease stack usage by Malloc replacement Use Malloc to allocate big local data structure to avoid stack overusage. Signed-off-by: Tomasz Gromadzki --- ChangeLog | 2 + src/common/mmap_posix.c | 13 ++++- src/common/set.c | 86 +++++++++++++++++++----------- src/stats/stack-usage-debug.txt | 14 ++--- src/stats/stack-usage-nondebug.txt | 17 +++--- 5 files changed, 83 insertions(+), 49 deletions(-) diff --git a/ChangeLog b/ChangeLog index b2df03d8f63..5a8598144e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,8 @@ This release: - Significantly reduces the libpmem's stack usage. + - Use Malloc for big local data structure to avoid huge stack usage + Tue Aug 8 2023 Oksana Sałyk diff --git a/src/common/mmap_posix.c b/src/common/mmap_posix.c index 29d45a19e41..143b9d1105f 100644 --- a/src/common/mmap_posix.c +++ b/src/common/mmap_posix.c @@ -11,6 +11,7 @@ #include "mmap.h" #include "out.h" #include "os.h" +#include "alloc.h" #define PROCMAXLEN 2048 /* maximum expected line length in /proc files */ @@ -31,6 +32,8 @@ static const char * const sscanf_os = "%p-%p"; * Asking for aligned address like this will allow the DAX code to use large * mappings. It is not an error if mmap() ignores the hint and chooses * different address. + * + * Return MAP_FAILED in case of an error. */ char * util_map_hint_unused(void *minaddr, size_t len, size_t align) @@ -44,11 +47,18 @@ util_map_hint_unused(void *minaddr, size_t len, size_t align) return MAP_FAILED; } - char line[PROCMAXLEN]; /* for fgets() */ + char *line = NULL; /* for fgets() */ char *lo = NULL; /* beginning of current range in maps file */ char *hi = NULL; /* end of current range in maps file */ char *raddr = minaddr; /* ignore regions below 'minaddr' */ + line = Malloc(PROCMAXLEN); + if (line == NULL) { + ERR("!Malloc"); + errno = ENOMEM; + return MAP_FAILED; + } + if (raddr == NULL) raddr += Pagesize; @@ -93,6 +103,7 @@ util_map_hint_unused(void *minaddr, size_t len, size_t align) fclose(fp); + Free(line); LOG(3, "returning %p", raddr); return raddr; } diff --git a/src/common/set.c b/src/common/set.c index 1cd908eb14c..56316d06aed 100644 --- a/src/common/set.c +++ b/src/common/set.c @@ -1791,46 +1791,60 @@ util_header_check(struct pool_set *set, unsigned repidx, unsigned partidx, ASSERTne(attr, NULL); + int ret = 0; + struct pool_replica *rep = set->replica[repidx]; /* opaque info lives at the beginning of mapped memory pool */ struct pool_hdr *hdrp = rep->part[partidx].hdr; - struct pool_hdr hdr; + struct pool_hdr *hdr; + + hdr = Malloc(sizeof(struct pool_hdr)); + if (hdr == NULL) { + ERR("!Malloc"); + errno = ENOMEM; + return -1; + } - memcpy(&hdr, hdrp, sizeof(hdr)); + memcpy(hdr, hdrp, sizeof(*hdr)); - util_convert2h_hdr_nocheck(&hdr); + util_convert2h_hdr_nocheck(hdr); /* to be valid, a header must have a major version of at least 1 */ - if (hdr.major == 0) { + if (hdr->major == 0) { ERR("invalid major version (0)"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check signature */ - if (memcmp(hdr.signature, attr->signature, POOL_HDR_SIG_LEN)) { - ERR("wrong pool type: \"%.8s\"", hdr.signature); + if (memcmp(hdr->signature, attr->signature, POOL_HDR_SIG_LEN)) { + ERR("wrong pool type: \"%.8s\"", hdr->signature); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check format version number */ - if (hdr.major != attr->major) { - ERR("pool version %d (library expects %d)", hdr.major, + if (hdr->major != attr->major) { + ERR("pool version %d (library expects %d)", hdr->major, attr->major); - if (hdr.major < attr->major) + if (hdr->major < attr->major) ERR( "Please run the pmdk-convert utility to upgrade the pool."); errno = EINVAL; - return -1; + ret = -1; + goto end; } rep->part[partidx].rdonly = 0; - int retval = util_feature_check(&hdr, attr->features); - if (retval < 0) - return -1; + int retval = util_feature_check(hdr, attr->features); + if (retval < 0) { + ret = -1; + goto end; + } if (retval == 0) rep->part[partidx].rdonly = 1; @@ -1843,44 +1857,49 @@ util_header_check(struct pool_set *set, unsigned repidx, unsigned partidx, * we want to report it as incompatible feature, rather than * invalid checksum. */ - if (!util_checksum(&hdr, sizeof(hdr), &hdr.checksum, - 0, POOL_HDR_CSUM_END_OFF(&hdr))) { + if (!util_checksum(hdr, sizeof(*hdr), &hdr->checksum, + 0, POOL_HDR_CSUM_END_OFF(hdr))) { ERR("invalid checksum of pool header"); errno = EINVAL; - return -1; + ret = -1; + goto end; } - LOG(3, "valid header, signature \"%.8s\"", hdr.signature); + LOG(3, "valid header, signature \"%.8s\"", hdr->signature); - if (util_check_arch_flags(&hdr.arch_flags)) { + if (util_check_arch_flags(&hdr->arch_flags)) { ERR("wrong architecture flags"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check pool set UUID */ - if (memcmp(HDR(REP(set, 0), 0)->poolset_uuid, hdr.poolset_uuid, + if (memcmp(HDR(REP(set, 0), 0)->poolset_uuid, hdr->poolset_uuid, POOL_HDR_UUID_LEN)) { ERR("wrong pool set UUID"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check pool set linkage */ - if (memcmp(HDRP(rep, partidx)->uuid, hdr.prev_part_uuid, + if (memcmp(HDRP(rep, partidx)->uuid, hdr->prev_part_uuid, POOL_HDR_UUID_LEN) || - memcmp(HDRN(rep, partidx)->uuid, hdr.next_part_uuid, + memcmp(HDRN(rep, partidx)->uuid, hdr->next_part_uuid, POOL_HDR_UUID_LEN)) { ERR("wrong part UUID"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check format version */ if (HDR(rep, 0)->major != hdrp->major) { ERR("incompatible pool format"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check compatibility features */ @@ -1889,15 +1908,20 @@ util_header_check(struct pool_set *set, unsigned repidx, unsigned partidx, HDR(rep, 0)->features.ro_compat != hdrp->features.ro_compat) { ERR("incompatible feature flags"); errno = EINVAL; - return -1; + ret = -1; + goto end; } /* check poolset options */ if (util_poolset_check_header_options(set, - HDR(rep, 0)->features.incompat)) - return -1; + HDR(rep, 0)->features.incompat)) { + ret = -1; + goto end; + } +end: + Free(hdr); - return 0; + return ret; } /* diff --git a/src/stats/stack-usage-debug.txt b/src/stats/stack-usage-debug.txt index 5b857ae20a4..2e0069baf7c 100644 --- a/src/stats/stack-usage-debug.txt +++ b/src/stats/stack-usage-debug.txt @@ -182,9 +182,6 @@ 4288 ndctl_match_devdax : src/debug/libpmemobj/region_namespace_ndctl.su:region_namespace_ndctl.c dynamic,bounded 4288 ndctl_match_devdax : src/debug/libpmem2/region_namespace_ndctl.su:region_namespace_ndctl.c dynamic,bounded 4288 ndctl_match_devdax : src/debug/common/region_namespace_ndctl.su:region_namespace_ndctl.c dynamic,bounded -4208 util_header_check : src/debug/libpmempool/set.su:set.c dynamic,bounded -4208 util_header_check : src/debug/libpmemobj/set.su:set.c dynamic,bounded -4208 util_header_check : src/debug/common/set.su:set.c dynamic,bounded 4176 features_check : src/debug/libpmempool/feature.su:feature.c static 4160 pool_hdr_default_fix : src/debug/libpmempool/check_pool_hdr.su:check_pool_hdr.c dynamic,bounded 4160 pmem2_deep_flush_write : src/debug/libpmempool/deep_flush_linux.su:deep_flush_linux.c static @@ -231,10 +228,6 @@ 2704 memset_movnt_movdir64b_clflush : src/debug/libpmem2/memset_nt_movdir64b.su:memset_nt_movdir64b.c dynamic,bounded 2704 memset_movnt_movdir64b_clflushopt : src/debug/libpmem/memset_nt_movdir64b.su:memset_nt_movdir64b.c dynamic,bounded 2704 memset_movnt_movdir64b_clflushopt : src/debug/libpmem2/memset_nt_movdir64b.su:memset_nt_movdir64b.c dynamic,bounded -2144 util_map_hint_unused : src/debug/libpmempool/mmap_posix.su:mmap_posix.c dynamic,bounded -2144 util_map_hint_unused : src/debug/libpmemobj/mmap_posix.su:mmap_posix.c dynamic,bounded -2144 util_map_hint_unused : src/debug/libpmem/mmap_posix.su:mmap_posix.c dynamic,bounded -2144 util_map_hint_unused : src/debug/common/mmap_posix.su:mmap_posix.c dynamic,bounded 1056 heap_write_header : src/debug/libpmemobj/heap.su:heap.c static 864 ulog_entry_buf_create : src/debug/libpmemobj/ulog.su:ulog.c dynamic 864 memblock_run_init : src/debug/libpmemobj/memblock.su:memblock.c dynamic,bounded @@ -769,6 +762,9 @@ 144 util_map : src/debug/libpmemobj/mmap.su:mmap.c dynamic,bounded 144 util_map : src/debug/libpmem/mmap.su:mmap.c dynamic,bounded 144 util_map : src/debug/common/mmap.su:mmap.c dynamic,bounded +144 util_header_check : src/debug/libpmempool/set.su:set.c dynamic,bounded +144 util_header_check : src/debug/libpmemobj/set.su:set.c dynamic,bounded +144 util_header_check : src/debug/common/set.su:set.c dynamic,bounded 144 ulog_entry_val_create : src/debug/libpmemobj/ulog.su:ulog.c static 144 tx_flush_range : src/debug/libpmemobj/tx.su:tx.c static 144 tx_alloc_common : src/debug/libpmemobj/tx.su:tx.c dynamic,bounded @@ -886,6 +882,10 @@ 112 util_poolset_append_new_part : src/debug/libpmempool/set.su:set.c dynamic,bounded 112 util_poolset_append_new_part : src/debug/libpmemobj/set.su:set.c dynamic,bounded 112 util_poolset_append_new_part : src/debug/common/set.su:set.c dynamic,bounded +112 util_map_hint_unused : src/debug/libpmempool/mmap_posix.su:mmap_posix.c dynamic,bounded +112 util_map_hint_unused : src/debug/libpmemobj/mmap_posix.su:mmap_posix.c dynamic,bounded +112 util_map_hint_unused : src/debug/libpmem/mmap_posix.su:mmap_posix.c dynamic,bounded +112 util_map_hint_unused : src/debug/common/mmap_posix.su:mmap_posix.c dynamic,bounded 112 util_file_zero : src/debug/libpmempool/file.su:file.c dynamic,bounded 112 util_file_zero : src/debug/libpmemobj/file.su:file.c dynamic,bounded 112 util_file_zero : src/debug/libpmem/file.su:file.c dynamic,bounded diff --git a/src/stats/stack-usage-nondebug.txt b/src/stats/stack-usage-nondebug.txt index 7ac24525825..52cf1b92970 100644 --- a/src/stats/stack-usage-nondebug.txt +++ b/src/stats/stack-usage-nondebug.txt @@ -28,9 +28,6 @@ 4224 check_domain_in_region : src/nondebug/libpmem/auto_flush_linux.su:auto_flush_linux.c dynamic,bounded 4224 check_domain_in_region : src/nondebug/libpmem2/auto_flush_linux.su:auto_flush_linux.c dynamic,bounded 4224 check_domain_in_region : src/nondebug/common/auto_flush_linux.su:auto_flush_linux.c dynamic,bounded -4176 util_header_check : src/nondebug/libpmempool/set.su:set.c static -4176 util_header_check : src/nondebug/libpmemobj/set.su:set.c static -4176 util_header_check : src/nondebug/common/set.su:set.c static 4176 features_check : src/nondebug/libpmempool/feature.su:feature.c static 4176 check_pool_hdr_uuids : src/nondebug/libpmempool/check_pool_hdr.su:check_pool_hdr.c static 4176 check_pool_hdr : src/nondebug/libpmempool/check_pool_hdr.su:check_pool_hdr.c static @@ -44,10 +41,6 @@ 4112 pool_set_type : src/nondebug/libpmempool/pool.su:pool.c static 3840 memset_mov_avx512f_empty : src/nondebug/libpmem/memset_t_avx512f.su:memset_t_avx512f.c static 3840 memset_mov_avx512f_empty : src/nondebug/libpmem2/memset_t_avx512f.su:memset_t_avx512f.c static -2144 util_map_hint_unused : src/nondebug/libpmempool/mmap_posix.su:mmap_posix.c static -2144 util_map_hint_unused : src/nondebug/libpmemobj/mmap_posix.su:mmap_posix.c static -2144 util_map_hint_unused : src/nondebug/libpmem/mmap_posix.su:mmap_posix.c static -2144 util_map_hint_unused : src/nondebug/common/mmap_posix.su:mmap_posix.c static 2016 memmove_mov_avx_empty : src/nondebug/libpmem/memcpy_t_avx.su:memcpy_t_avx.c static 2016 memmove_mov_avx_empty : src/nondebug/libpmem2/memcpy_t_avx.su:memcpy_t_avx.c static 1184 heap_init : src/nondebug/libpmemobj/heap.su:heap.c static @@ -230,6 +223,9 @@ 208 pmem2_vm_reservation_map_find : src/nondebug/libpmem2/vm_reservation.su:vm_reservation.c dynamic,bounded 208 memblock_huge_init : src/nondebug/libpmemobj/memblock.su:memblock.c static 208 list_insert : src/nondebug/libpmemobj/list.su:list.c static +192 util_pool_open : src/nondebug/libpmempool/set.su:set.c static +192 util_pool_open : src/nondebug/libpmemobj/set.su:set.c static +192 util_pool_open : src/nondebug/common/set.su:set.c static 192 util_map_part : src/nondebug/libpmempool/set.su:set.c dynamic,bounded 192 util_map_part : src/nondebug/libpmemobj/set.su:set.c dynamic,bounded 192 util_map_part : src/nondebug/common/set.su:set.c dynamic,bounded @@ -283,9 +279,6 @@ 192 heap_get_bestfit_block : src/nondebug/libpmemobj/heap.su:heap.c static 184 operation_init : src/nondebug/libpmemobj/memops.su:memops.c static 176 vm_reservation_map_find_acquire : src/nondebug/libpmem2/vm_reservation.su:vm_reservation.c static -176 util_pool_open : src/nondebug/libpmempool/set.su:set.c static -176 util_pool_open : src/nondebug/libpmemobj/set.su:set.c static -176 util_pool_open : src/nondebug/common/set.su:set.c static 176 util_map_hdr : src/nondebug/libpmempool/set.su:set.c dynamic,bounded 176 util_map_hdr : src/nondebug/libpmemobj/set.su:set.c dynamic,bounded 176 util_map_hdr : src/nondebug/common/set.su:set.c dynamic,bounded @@ -468,6 +461,10 @@ 96 util_pool_extend : src/nondebug/libpmempool/set.su:set.c static 96 util_pool_extend : src/nondebug/libpmemobj/set.su:set.c static 96 util_pool_extend : src/nondebug/common/set.su:set.c static +96 util_map_hint_unused : src/nondebug/libpmempool/mmap_posix.su:mmap_posix.c static +96 util_map_hint_unused : src/nondebug/libpmemobj/mmap_posix.su:mmap_posix.c static +96 util_map_hint_unused : src/nondebug/libpmem/mmap_posix.su:mmap_posix.c static +96 util_map_hint_unused : src/nondebug/common/mmap_posix.su:mmap_posix.c static 96 ulog_free_next : src/nondebug/libpmemobj/ulog.su:ulog.c static 96 tx_undo_entry_apply : src/nondebug/libpmemobj/tx.su:tx.c static 96 tx_flush_range : src/nondebug/libpmemobj/tx.su:tx.c static