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.

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

Change path max stack allocation to heap #10127

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2a83842
UCS/MEMTRACK: initialize VFS before enabling memtrack and cleanup aft…
michal-shalev Aug 20, 2024
533b10e
UCS/LOG: convert stack allocation at ucs_log_file_rotate() to heap
michal-shalev Jun 17, 2024
07b11f0
UCS/VFS: convert stack allocation at ucs_vfs_node_add() to heap
michal-shalev Jun 18, 2024
20e3aca
UCS/MEMORY/NUMA: convert stack allocation at ucs_numa_node_of_cpu() t…
michal-shalev Aug 28, 2024
d1ed141
UCS/PARSER: convert stack allocation at ucs_config_parse_config_files…
michal-shalev Aug 27, 2024
abe248e
UCS/PARSER: convert stack allocation at ucs_config_parse_config_file(…
michal-shalev Aug 27, 2024
dcc406a
UCS/STRING/GTEST: convert stack allocation at test_string path to heap
michal-shalev Aug 28, 2024
5f30717
UCS/VFS/SOCK: convert stack allocation at ucs_vfs_sock_mkdir() to heap
michal-shalev Aug 29, 2024
84f8d0f
UCS/VFS/BASE: convert stack allocation at ucs_vfs_node_add_subdir() t…
michal-shalev Aug 29, 2024
bdf20a9
UCS/VFS/FUSE: convert stack allocation at ucs_vfs_fuse_wait_for_path(…
michal-shalev Sep 6, 2024
80e54aa
UCS/SYS/SOCK: convert stack allocation at ucs_netif_bond_ad_num_ports…
michal-shalev Aug 29, 2024
8bf1be4
UCS/SYS/MODULE: convert stack allocation at ucs_module_load_one() to …
michal-shalev Jun 24, 2024
ffa0f80
UCS/SYS/MODULE: convert stack allocation at ucs_module_init() to heap
michal-shalev Sep 16, 2024
b4e2ef9
UCT/SM/MM: convert stack allocation at uct_posix_file_open() to heap
michal-shalev Aug 28, 2024
903e27d
UCT/SM/MM: convert stack allocation at uct_posix_procfs_open() to heap
michal-shalev Aug 28, 2024
10887ae
UCT/SM/MM: convert stack allocation at uct_posix_unlink() to heap
michal-shalev Aug 28, 2024
ca52631
UCT/TCP/BASE: convert stack allocation at uct_tcp_iface_get_sysfs_pat…
michal-shalev Aug 29, 2024
a1af73b
UCT/TCP: convert stack allocation at uct_tcp_iface_query() to heap
michal-shalev Jun 24, 2024
77945e8
UCT/TCP: convert stack allocation at uct_tcp_is_bridge() to heap
michal-shalev Sep 6, 2024
5530e3f
UCT/IB/BASE: convert stack allocation at uct_ib_device_query() to heap
michal-shalev Aug 29, 2024
683f4e2
UCP/WORKER/GTEST: convert stack allocation at test_pci_bw get_pci_bw …
michal-shalev Aug 29, 2024
d7ac06a
UCP/PROTO/DEBUG: convert stack allocation at ucp_proto_select_write_i…
michal-shalev Aug 29, 2024
c268306
COMMON/HELPERS/GTEST: convert stack allocation at netif_has_sysfs_fil…
michal-shalev Aug 29, 2024
422a782
TOOLS/VFS: convert stack allocation at vfs_mount() to heap
michal-shalev Sep 6, 2024
27e63b9
TOOLS/VFS: convert stack allocation at vfs_unmount() to heap
michal-shalev Sep 16, 2024
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
48 changes: 37 additions & 11 deletions src/tools/vfs/vfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

#include "vfs_daemon.h"

#include <ucs/sys/string.h>
#include <ucs/debug/log_def.h>
#include <ucs/debug/memtrack_int.h>
#include <sys/wait.h>
#include <limits.h>
#include <stdlib.h>
Expand Down Expand Up @@ -151,10 +153,17 @@ static const char *vfs_get_process_name(int pid, char *buf, size_t max_length)

int vfs_mount(int pid)
{
char mountpoint[PATH_MAX];
char mountopts[1024];
char name[NAME_MAX];
char *mountpoint;
int fuse_fd, ret;
ucs_status_t status;

status = ucs_string_alloc_path_buffer(&mountpoint, "mountpoint");
if (status != UCS_OK) {
ret = -ENOMEM;
goto out;
}

/* Add common mount options:
* - File system name (source) : process name and pid
Expand All @@ -168,58 +177,75 @@ int vfs_mount(int pid)
vfs_get_process_name(pid, name, sizeof(name)),
(strlen(g_opts.mount_opts) > 0) ? "," : "", g_opts.mount_opts);
if (ret >= sizeof(mountopts)) {
return -ENOMEM;
ret = -ENOMEM;
goto out_free_mountpoint;
}

/* Create the mount point directory, and ignore "already exists" error */
vfs_get_mountpoint(pid, mountpoint, sizeof(mountpoint));
vfs_get_mountpoint(pid, mountpoint, PATH_MAX);
ret = mkdir(mountpoint, S_IRWXU);
if ((ret < 0) && (errno != EEXIST)) {
ret = -errno;
vfs_error("failed to create directory '%s': %m", mountpoint);
return ret;
goto out_free_mountpoint;
}

/* Mount a new FUSE filesystem in the mount point directory */
vfs_log("mounting directory '%s' with options '%s'", mountpoint, mountopts);
fuse_fd = fuse_open_channel(mountpoint, mountopts);
if (fuse_fd < 0) {
ret = fuse_fd;
Comment on lines 195 to +197
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fuse_fd = fuse_open_channel(mountpoint, mountopts);
if (fuse_fd < 0) {
ret = fuse_fd;
ret = fuse_open_channel(mountpoint, mountopts);
if (ret< 0) {

vfs_error("fuse_open_channel(%s,opts=%s) failed: %m", mountpoint,
mountopts);
return fuse_fd;
goto out_free_mountpoint;
}

vfs_log("mounted directory '%s' with fd %d", mountpoint, fuse_fd);
return fuse_fd;
ret = fuse_fd;

out_free_mountpoint:
ucs_free(mountpoint);
out:
return ret;
}

int vfs_unmount(int pid)
{
char mountpoint[PATH_MAX];
char *mountpoint;
char *argv[5];
int ret;
ucs_status_t status;

status = ucs_string_alloc_path_buffer(&mountpoint, "mountpoint");
if (status != UCS_OK) {
ret = -ENOMEM;
goto out;
}

/* Unmount FUSE file system */
vfs_get_mountpoint(pid, mountpoint, sizeof(mountpoint));
vfs_get_mountpoint(pid, mountpoint, PATH_MAX);
argv[0] = "-u";
argv[1] = "-z";
argv[2] = "--";
argv[3] = mountpoint;
argv[4] = NULL;
ret = vfs_run_fusermount(argv);
if (ret < 0) {
return ret;
goto out_free_mountpoint;
}

/* Remove mount point directory */
vfs_log("removing directory '%s'", mountpoint);
ret = rmdir(mountpoint);
if (ret < 0) {
vfs_error("failed to remove directory '%s': %m", mountpoint);
return ret;
goto out_free_mountpoint;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why need goto?

}

return 0;
out_free_mountpoint:
ucs_free(mountpoint);
out:
return ret;
}

static int vfs_unlink_socket(int silent_notexist)
Expand Down
19 changes: 15 additions & 4 deletions src/ucp/proto/proto_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -984,24 +984,30 @@ ucp_proto_select_write_info(ucp_worker_h worker,
ucp_proto_query_attr_t proto_attr;
size_t range_start, range_end;
ucs_string_buffer_t dot_strb;
char dir_path[PATH_MAX];
ucs_status_t status;
char *dir_path;
FILE *fp;
int ret;

status = ucs_string_alloc_path_buffer(&dir_path, "dir_path");
if (status != UCS_OK) {
goto out;
}

ucp_proto_select_param_dump(worker, ep_cfg_index, rkey_cfg_index,
select_param, ucp_operation_names, &ep_cfg_strb,
&sel_param_strb);
if (!ucp_proto_debug_is_info_enabled(
worker->context, ucs_string_buffer_cstr(&sel_param_strb))) {
return;
goto out_free_dir_path;
}

ucs_fill_filename_template(worker->context->config.ext.proto_info_dir,
dir_path, sizeof(dir_path));
dir_path, PATH_MAX);
ret = mkdir(dir_path, S_IRWXU | S_IRGRP | S_IXGRP);
if ((ret != 0) && (errno != EEXIST)) {
ucs_debug("failed to create directory %s: %m", dir_path);
return;
goto out_free_dir_path;
}

ucs_string_buffer_translate(&ep_cfg_strb, ucp_proto_debug_fix_filename);
Expand Down Expand Up @@ -1040,6 +1046,11 @@ ucp_proto_select_write_info(ucp_worker_h worker,
fclose(fp);

} while (range_end != SIZE_MAX);

out_free_dir_path:
ucs_free(dir_path);
out:
return;
}

void ucp_proto_select_elem_trace(ucp_worker_h worker,
Expand Down
28 changes: 25 additions & 3 deletions src/ucs/config/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -1614,15 +1614,21 @@ void ucs_config_parse_config_file(const char *dir_path, const char *file_name,
.section_info = {.name = "",
.skip = 0}
};
char file_path[MAXPATHLEN];
char *file_path;
int parse_result;
FILE* file;
ucs_status_t status;

status = ucs_string_alloc_path_buffer(&file_path, "file_path");
if (status != UCS_OK) {
goto out;
}

ucs_snprintf_safe(file_path, MAXPATHLEN, "%s/%s", dir_path, file_name);
file = fopen(file_path, "r");
if (file == NULL) {
ucs_debug("failed to open config file %s: %m", file_path);
return;
goto out_free_file_path;
}

parse_result = ini_parse_file(file, ucs_config_parse_config_file_line,
Expand All @@ -1633,6 +1639,11 @@ void ucs_config_parse_config_file(const char *dir_path, const char *file_name,

ucs_debug("parsed config file %s", file_path);
fclose(file);

out_free_file_path:
ucs_free(file_path);
out:
return;
}

static ucs_status_t
Expand Down Expand Up @@ -1750,8 +1761,14 @@ static ucs_status_t ucs_config_parser_get_sub_prefix(const char *env_prefix,

void ucs_config_parse_config_files()
{
char *path;
const char *path_p;
char path[PATH_MAX];
ucs_status_t status;

status = ucs_string_alloc_path_buffer(&path, "path");
if (status != UCS_OK) {
goto out;
}

/* System-wide configuration file */
ucs_config_parse_config_file(UCX_CONFIG_DIR, UCX_CONFIG_FILE_NAME, 1);
Expand All @@ -1778,6 +1795,11 @@ void ucs_config_parse_config_files()

/* Current working dir */
ucs_config_parse_config_file(".", UCX_CONFIG_FILE_NAME, 1);

out_free_path:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused label (pls check other places too)?

ucs_free(path);
out:
return;
}

ucs_status_t
Expand Down
29 changes: 24 additions & 5 deletions src/ucs/debug/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,27 @@ static void ucs_log_get_file_name(char *log_file_name, size_t max, int idx)

static void ucs_log_file_rotate()
{
char old_log_file_name[PATH_MAX];
char new_log_file_name[PATH_MAX];
char *old_log_file_name, *new_log_file_name;
int idx, ret;
ucs_status_t status;

status = ucs_string_alloc_path_buffer(&old_log_file_name,
"old_log_file_name");
if (status != UCS_OK) {
goto out;
}

status = ucs_string_alloc_path_buffer(&new_log_file_name,
"new_log_file_name");
if (status != UCS_OK) {
goto out_free_old_log_file_name;
}

if (ucs_log_file_last_idx == ucs_global_opts.log_file_rotate) {
/* remove the last file and log rotation from the
* `log_file_rotate - 1` file */
ucs_log_get_file_name(old_log_file_name,
sizeof(old_log_file_name),
PATH_MAX,
ucs_log_file_last_idx);
unlink(old_log_file_name);
} else {
Expand All @@ -164,9 +176,9 @@ static void ucs_log_file_rotate()

for (idx = ucs_log_file_last_idx - 1; idx >= 0; --idx) {
ucs_log_get_file_name(old_log_file_name,
sizeof(old_log_file_name), idx);
PATH_MAX, idx);
ucs_log_get_file_name(new_log_file_name,
sizeof(new_log_file_name), idx + 1);
PATH_MAX, idx + 1);

if (access(old_log_file_name, W_OK) != 0) {
ucs_fatal("unable to write to %s", old_log_file_name);
Expand All @@ -188,6 +200,13 @@ static void ucs_log_file_rotate()
ucs_fatal("unable to write to %s", new_log_file_name);
}
}

out_free_new_log_file_name:
ucs_free(new_log_file_name);
out_free_old_log_file_name:
ucs_free(old_log_file_name);
out:
return;
}

static void ucs_log_handle_file_max_size(int log_entry_len)
Expand Down
9 changes: 4 additions & 5 deletions src/ucs/debug/memtrack.c
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,10 @@ void ucs_memtrack_init()
return;
}

ucs_memtrack_vfs_init();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that change related to this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I changed allocation at VFS to heap and it revealed this issue - an invalid count in the memtrack (tests were failing)


ucs_debug("memtrack enabled");
ucs_memtrack_context.enabled = 1;

ucs_memtrack_vfs_init();
}

void ucs_memtrack_cleanup()
Expand All @@ -447,12 +447,11 @@ void ucs_memtrack_cleanup()
return;
}

ucs_vfs_obj_remove(&ucs_memtrack_context);

ucs_memtrack_generate_report();

/* disable before releasing the stats node */
/* disable before releasing the vfs object and the stats node */
ucs_memtrack_context.enabled = 0;
ucs_vfs_obj_remove(&ucs_memtrack_context);
UCS_STATS_NODE_FREE(ucs_memtrack_context.stats);

/* cleanup entries */
Expand Down
17 changes: 13 additions & 4 deletions src/ucs/memory/numa.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,25 @@ ucs_numa_node_t ucs_numa_node_of_cpu(int cpu)
{
/* Used for caching to improve performance */
static ucs_numa_node_t cpu_numa_node[__CPU_SETSIZE] = {0};
char *core_dir_path;
ucs_numa_node_t node;
char core_dir_path[PATH_MAX];
ucs_status_t status;

ucs_assert(cpu < __CPU_SETSIZE);

if (cpu_numa_node[cpu] == 0) {
status = ucs_string_alloc_path_buffer(&core_dir_path, "core_dir_path");
if (status != UCS_OK) {
return UCS_NUMA_NODE_UNDEFINED;
}

ucs_snprintf_safe(core_dir_path, PATH_MAX, UCS_NUMA_CORE_DIR_PATH, cpu);
node = ucs_numa_get_max_dirent(core_dir_path, "node",
ucs_numa_num_configured_nodes(),
UCS_NUMA_NODE_DEFAULT);
node = ucs_numa_get_max_dirent(core_dir_path, "node",
ucs_numa_num_configured_nodes(),
UCS_NUMA_NODE_DEFAULT);

ucs_free(core_dir_path);

cpu_numa_node[cpu] = node + 1;
}

Expand Down
Loading
Loading