In short applications, it's possible that the agent (i.e., local rank
0) will finalize after non-local rank 0 procs detect the connectivity
checker named socket, but before they complete a connect() on it. As
such, their connect() gets ECONNREFUSED.
This commit adds a simple counter in the agent that won't let it quit
before it accept()'s from all local procs, or 10 seconds goes by
(whichever occurs first). This is similar to the timeout for the
clients: they'll exit if they don't see the expected named socket
within 10 seconds.
There's no longer any need for the usnic BTL to have its own progress
thread: it can use the opal_progress_thread() infrastructure. This
commit removes the code to startup/shutdown the usnic-BTL-specific
progress thread and instead, just adds its events to the OPAL-wide
progress thread.
This necessitated a small change in the finalization step.
Previously, we would stop the progress thread and then tear down the
events. We can no longer stop the progress thread, and if we start
tearing down events, this will cause shutdown/hangups to be sent
across sockets, potentially firing some of the still-remaining events
while some (but not all) of the data structures have been torn down.
Chaos ensues.
Instead, queue up an event to tear down all the pending events. Since
the progress thread will only fire one event at a time, having a
teardown event means that it can tear down all the pending events
"atomically" and not have to worry that one of those events will get
fired in the middle of the teardown process.
Ensure that we have non-NULL on all levels of pointers, which will
save us if there are exitable errors very early during component /
module initialization.
since ibv_create_xrc_rcv_qp is now deprecated, and in order to
be "future-proof", we have to consider the case in which only XRC Domains are supported.
also, correctly handle distro that ship broken ibverbs devel headers
Thanks Paul Hargrove for the detailled report.
In libfabric v1.0.0 (i.e., API v1.0), the usnic provider handled
FI_MSG_PREFIX inconsistently between sends and receives. This has
been fixed in libfabric v1.1.0 (i.e., API v1.1): FI_MSG_PREFIX is
handled consistently for both sends and receives.
Run-time detect which libfabric we are running with and adapt behavior
appropriately.
Handle the differences between libfabric v1.0.0 and v1.1.0 in the
return value of fi_cq_readerr().
Also consolidate CRC and truncation errors into the same handling
block, since truncation errors are typically another symptom of CRC
errors. This ensures that buffers get reposted properly.
Instead of silently determining that the usnic BTL can't be built,
announce that usnic is checking for libfabric support, and then
AC_MSG_RESULT the result of that check.
@ggouaillardet is likely offline for the weekend, but master is broken
on RHEL 6.5 systems that do not have MOFED installed. So I'm taking
the liberty of revering this commit; I'm guessing Gilles will fixup
and re-commit next week.
This reverts commit 77f8282d51.
since ibv_create_xrc_rcv_qp is now deprecated, and in order to
be "future-proof", we have to consider the case in which only XRC Domains are supported.
Thanks Paul Hargrove for the detailled report.
The usnic BTL configure.m4 no longer needs to OPAL_CHECK_LIBFABRIC; it
just uses the results from opal/mca/common/libfabric's configure.m4.
We also now don't need to link against libfabric -- they just link
against the opal_common_libfabric library.
This commit does two things. It removes checks for C99 required
headers (stdlib.h, string.h, signal.h, etc). Additionally it removes
definitions for required C99 types (intptr_t, int64_t, int32_t, etc).
Signed-off-by: Nathan Hjelm <hjelmn@me.com>
Minor comment/whitespace fixes. Also some minor logic changes that
are mainly for defensive programming purposes (i.e., ensure to always
set malloc_hook_set to true or false, and then check it before we try
to actually invoke it).
Instead of unconditionally setting the memory hook, only set it when
the memory hooks are both available and have been enabled (e.g.,
opal/mca/memory/linux has decided that it *can* be enabled, and when
the mpi_leave_pinned MCA param is set to 1, or is set to -1 and some
component requested the memory hooks be enabled).
If we set the memory hook when memory hooks are not enabled,
__malloc_hook will be NULL, which will cause problems when
btl_openib_malloc_hook() tries to invoke it.
Fixesopen-mpi/ompi#638.
Also add another (superflous but symmetric) continue statement.
This missing "continue" statement allows IPv4 "private network"
matches to fall through and allow IPv6 matches to be made -- thereby
overriding the IPv4 match that was already made.
Fixes#585 (although several of the other issues identified on #585
still exist, the primary / initial bug that was reported there is now
fixed).
CID 1269821 Dereference null return value (NULL_RETURNS)
This is another false positive that can be silenced by looping on
opal_list_remove_first instead of using both opal_list_is_empty and
opal_list_remove_first.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1301390 Dereference before null check (REVERSE_INULL)
endpoint can not be NULL here. Remove NULL check.
CID 1269836 Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
CID 1301388 Bad bit shift operation (BAD_SHIFT)
Add ull to integer constants to ensure the math is done in 64-bits not
32.
CID 715749 Explicit null dereferenced (FORWARD_NULL)
As far as I can tell this parser function does not accept a line that
does match key = value. If that is the case then value should never be
NULL. If it is it is a parse error. Updated the code to reflect
this. Also modified the intify function to do something more sane
(strtol vs atoi with hex detection).
CID 1269820 Dereference null return value (NULL_RETURNS)
This is a false positive as strchr will never return NULL here. It
makes sense, though, to quiet the warning by changing the do {} while
() loop to a while () loop.
CID 1269780 Dereference after null check (FORWARD_NULL)
Just return an error if the endpoint's cpc data is NULL.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269674 Ignoring number of bytes read (CHECKED_RETURN)
Check that we read enough bytes to get a complete async command.
CID 1269793 Missing break in switch (MISSING_BREAK)
Added comment to indicate fall through was intentional.
CID 1269702: Constant variable guards dead code (DEADCODE)
Remove an unused argument to opal_show_help. This will quiet the
coverity issue.
CID 1269675 Ignoring number of bytes read (CHECKED_RETURN)
Check that at least sizeof(int) bytes are read. If this is not the
case then it is an error.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269931 Uninitialized scalar variable (UNINIT)
Initialize complete async message. This was not a bug but the fix
contributes to valgrind cleanness (uninitialed write).
CID 1269915 Unintended sign extension (SIGN_EXTENSION)
Should never happen. Quieting this by explicitly casting to uint64_t.
CID 1269824 Dereference null return value (NULL_RETURNS)
It is impossible for opal_list_remove_first to return NULL if
opal_list_is_empty returns false. I refactored the code in question to
not use opal_list_is_empty but loop until NULL is returned by
opal_list_remove_first. That will quiet the issue.
CID 1269913 Dereference before null check (REVERSE_INULL)
The storage parameter should never be NULL. The check intended to
check if *storage was NULL not storage.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269933 Uninitialized scalar variable (UNINIT)
This CID isn't really an error but it is best for both valgrind and
coverity cleanness to not write uninitialized data. Added an
initializer for async_command in btl_openib_component_close.
CID 1269930 Uninitialized scalar variable (UNINIT)
Same as above. Best not to write uninitialized data. Added an
initializer for async_command.
CID 1269701 Logically dead code (DEADCODE)
Coverity is correct. The smallest_pp_qp will always be 0. Changed the
initial value so that the smallest_pp_qp is set as intended. If no
per-per queue pair exists then use the last shared queue pair. This
queue pair should have the smallest message size. This will reduce
buffer waste.
CID 1269713 Logically dead code (DEADCODE)
False positive but easy to silence. The two check are meaningless if
HAVE_XRC is 0 so protect them with #if HAVE_XRC.
CID 1269726 Division or modulo by zero (DIVIDE_BY_ZERO)
Indeed an issue. If we get an invalid value for rd_win then this will
cause a divide-by-zero exception. Added a check to ensure rd_win is >
0. Also updated the help message to reflect this requirement.
CID 1269672 Ignoring number of bytes read (CHECKED_RETURN)
This error was somewhat intentional. Linux parameter files are
probably not empty but it is safer to check the return code of read to
make sure we got something. If 0 bytes are read this code could SEGV
whe running strtoull.
CID 1269836 Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
Add a range check to read_module_param to ensure we do not
overflow. In the future it might be worthwhile to report an error
because these parameters should never cause overflow in this
calculation.
CID 1269692 Calling risky function (DC.WEAK_CRYPTO)
??? This call was added in 2006 but I see no calls to the rest of the
rand48 family of functions. Anyway, we SHOULD NEVER be calling seed48,
srand, etc because it messes with user code. Removed the call to
seed48.
CID 1269823 Dereference null return value (NULL_RETURNS)
This is likely a false positive. The endpoint lock is being held so no
other thread should be able to remove fragments from the list. Also,
mca_btl_openib_endpoint_post_send should not be removing items from
the list. If a NULL fragment is ever returned it will likely be a
coding error on the part of an Open MPI developer. Added an assert()
to catch this and quiet the coverity error.
CID 1269671 Unchecked return value (CHECKED_RETURN)
Added a check for the return code of mca_btl_openib_endpoint_post_send
to quiet the coverity error. It is unlikely this error path will be
traversed.
CID 1270229 Missing break in switch (MISSING_BREAK)
Add a comment to indicate that the fall-through is intentional.
CID 1269735 Dereference after null check (FORWARD_NULL)
There should always be an endpoint when handling a work
completion. The endpoint is either stored on the fragment or can be
looked up using the immediate data. Move the immediate data code up
and add an assert for a NULL endpoint.
CID 1269740 Dereference after null check (FORWARD_NULL)
CID 1269741 Explicit null dereferenced (FORWARD_NULL)
Similar to CID 1269735 fix.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
