-
Notifications
You must be signed in to change notification settings - Fork 306
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
DAOS-16645 cart: Bump file descriptor limit #15224
Changes from 6 commits
6f723ee
cce4e6d
c7f7403
6c49a8e
28b70ad
5df6142
772d239
861b4cf
e7c32be
d9cee76
c63e298
39bc7d9
9b5439d
2312afc
5449989
72bb00a
92c3afc
e85614a
10e786c
c80b533
63b1496
ed1d48a
bf5aee8
2465445
89f6a11
99dbd33
808030f
560f030
782c700
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -492,10 +492,14 @@ check_grpid(crt_group_id_t grpid) | |
return rc; | ||
} | ||
|
||
#define MIN_TCP_FD 131072 | ||
|
||
static void | ||
prov_settings_apply(bool primary, crt_provider_t prov, crt_init_options_t *opt) | ||
{ | ||
uint32_t mrc_enable = 0; | ||
struct rlimit rlim; | ||
int rc; | ||
uint32_t mrc_enable = 0; | ||
|
||
/* Avoid applying same settings multiple times */ | ||
if (g_prov_settings_applied[prov] == true) | ||
|
@@ -510,6 +514,35 @@ prov_settings_apply(bool primary, crt_provider_t prov, crt_init_options_t *opt) | |
d_setenv("FI_OFI_RXM_DEF_TCP_WAIT_OBJ", "pollfd", 0); | ||
} | ||
|
||
if (prov == CRT_PROV_OFI_TCP || prov == CRT_PROV_OFI_TCP_RXM) { | ||
/* Bump file descriptor limit if low and if possible */ | ||
rc = getrlimit(RLIMIT_NOFILE, &rlim); | ||
if (rc != 0) { | ||
DS_WARN(errno, "getrlimit() failed. Unable to check file descriptor limit"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this a warn ? to me it sounds like it should be an error with appropriate errno checking ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. Looking at the man page, it doesn't look like it should ever fail in our case. |
||
goto next; | ||
} | ||
|
||
if (rlim.rlim_cur < MIN_TCP_FD) { | ||
if (rlim.rlim_max < MIN_TCP_FD) { | ||
D_ERROR("File descriptor hard limit should be at least %d\n", | ||
MIN_TCP_FD); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider that as a warning, not as an error, if let's say someone wanted for any reason to have a lower limit... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, this is a warning - in effect it's asking the admin to reconfigure the node but there's no error here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
goto next; | ||
} | ||
|
||
rlim.rlim_cur = rlim.rlim_max; | ||
rc = setrlimit(RLIMIT_NOFILE, &rlim); | ||
if (rc != 0) { | ||
DS_ERROR(errno, | ||
"setrlimit() failed. Unable to bump file descriptor" | ||
" limit to value >= %d, limit is %lu", | ||
MIN_TCP_FD, rlim.rlim_max); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. goto next; missing here? ele on error you will print d_info below:) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done, good catch. I added the print later.
soumagne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
goto next; | ||
} | ||
D_INFO("Updated soft file descriptor limit to %lu\n", rlim.rlim_max); | ||
} | ||
} | ||
next: | ||
|
||
jolivier23 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (prov == CRT_PROV_OFI_CXI) | ||
mrc_enable = 1; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please prefix internal macros with CRT_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done