Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid stack usage for big structure like pool_hdr #5889

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Avoid stack usage for big structure like pool_hdr #5889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ChangeLog
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading