Skip to content

Commit

Permalink
core/util: Additional fixes for mr cache hooking mechanism
Browse files Browse the repository at this point in the history
This commit fixes:
   - removes unnecessary fastlock acquiring/release inside of
     ofi_intercept_symbol() function. We cannot acquire monitor lock
     inside of this function since it already acquired
     in ofi_monitor_add_cache();
   - adds missing dlist_init() for dl_intercept_list;
   - fixes dlist_insert() segfault in ofi_intercept_symbol();
   - adds missing logic for FI_MR_CACHE_MONITOR supporting "userfaultfd"
     and "memhooks monitor" with fallback to default monitor.
   - adds some missing cleanup logic

Signed-off-by: Mikhail Khalilov <mikhail.khalilov@intel.com>
  • Loading branch information
Mikhail Khalilov committed Jul 29, 2019
1 parent dbfd9a4 commit 0feb0c5
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
15 changes: 5 additions & 10 deletions prov/util/src/util_mem_hooks.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,16 +295,12 @@ static int ofi_intercept_symbol(struct ofi_intercept *intercept, void **real_fun
{
int ret;

/*
* Take lock first to handle a possible race where dlopen() is called
* from another thread and we may end up not patching it.
*/
FI_DBG(&core_prov, FI_LOG_MR,
"intercepting symbol %s\n", intercept->symbol);
fastlock_acquire(&memhooks_monitor->lock);
dlist_init(&intercept->dl_intercept_list);
ret = dl_iterate_phdr(ofi_intercept_phdr_handler, intercept);
if (ret)
goto unlock;
return ret;

*real_func = dlsym(RTLD_DEFAULT, intercept->symbol);
if (*real_func == intercept->our_func) {
Expand All @@ -316,11 +312,10 @@ static int ofi_intercept_symbol(struct ofi_intercept *intercept, void **real_fun
FI_DBG(&core_prov, FI_LOG_MR,
"could not find symbol %s\n", intercept->symbol);
ret = -FI_ENOMEM;
goto unlock;
return ret;
}
dlist_insert_tail(&memhooks.intercept_list, &intercept->entry);
unlock:
fastlock_release(&memhooks_monitor->lock);
dlist_insert_tail(&intercept->entry, &memhooks.intercept_list);

return ret;
}

Expand Down
19 changes: 14 additions & 5 deletions prov/util/src/util_mem_monitor.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

#include <ofi_mr.h>


static struct ofi_uffd uffd;
struct ofi_mem_monitor *uffd_monitor = &uffd.monitor;

Expand All @@ -53,9 +52,9 @@ void ofi_monitor_init(void)
dlist_init(&memhooks_monitor->list);

#if HAVE_UFFD_UNMAP
struct ofi_mem_monitor *default_monitor = uffd_monitor;
default_monitor = uffd_monitor;
#else
struct ofi_mem_monitor *default_monitor = memhooks_monitor;
default_monitor = memhooks_monitor;
#endif

fi_param_define(NULL, "mr_cache_max_size", FI_PARAM_SIZE_T,
Expand Down Expand Up @@ -93,6 +92,14 @@ struct ofi_mem_monitor *default_monitor = memhooks_monitor;

if (!cache_params.max_size)
cache_params.max_size = SIZE_MAX;

if(cache_params.monitor != NULL) {
if (!strcmp(cache_params.monitor, "userfaultfd") &&
default_monitor == uffd_monitor) /* check that userfaultfd supported at all */
default_monitor = uffd_monitor;
else if (!strcmp(cache_params.monitor, "memhooks"))
default_monitor = memhooks_monitor;
}
}

void ofi_monitor_cleanup(void)
Expand Down Expand Up @@ -140,8 +147,10 @@ void ofi_monitor_del_cache(struct ofi_mr_cache *cache)
dlist_remove(&cache->notify_entry);

if (dlist_empty(&monitor->list)) {
ofi_uffd_cleanup();
ofi_memhooks_cleanup();
if (monitor == uffd_monitor)
ofi_uffd_cleanup();
else if (monitor == memhooks_monitor)
ofi_memhooks_cleanup();
}

fastlock_release(&monitor->lock);
Expand Down

0 comments on commit 0feb0c5

Please sign in to comment.