Skip to content

Commit

Permalink
common: decrease stack usage by Malloc replacement
Browse files Browse the repository at this point in the history 
Use Malloc to allocate big local data structure to avoid
stack overusage.

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
  • Loading branch information
@grom72
grom72 committed Oct 24, 2023
1 parent 87108b8 commit d1f1290
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 49 deletions.
2 changes: 2 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <oksana.salyk@intel.com>

Expand Down
13 changes: 12 additions & 1 deletion src/common/mmap_posix.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -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)
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
86 changes: 55 additions & 31 deletions src/common/set.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 */
Expand All @@ -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;
}

/*
Expand Down
14 changes: 7 additions & 7 deletions src/stats/stack-usage-debug.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 7 additions & 10 deletions src/stats/stack-usage-nondebug.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d1f1290

Please sign in to comment.