Skip to content

Commit

Permalink
util/mr_cache: Defer MR cleanup to flush operation
Browse files Browse the repository at this point in the history
We cannot safely hold the monitor lock while freeing
memory or deregistering a MR.  Doing so can result in
hanging the local thread, as well as the uffd thread.
When uncaching a region, instead of removing it
immediately if the use_cnt is 0, instead queue it to
a flush_list associated with the MR cache.

When calling ofi_mr_cache_flush, always cleanup the
flush_list, without holding the monitor lock.

Signed-off-by: Sean Hefty <sean.hefty@intel.com>
  • Loading branch information
shefty committed Jan 22, 2020
1 parent 2795e94 commit 3d01df7
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 21 deletions.
3 changes: 2 additions & 1 deletion include/ofi_mr.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ struct ofi_mr_entry {
void *storage_context;
unsigned int subscribed:1;
int use_cnt;
struct dlist_entry lru_entry;
struct dlist_entry list_entry;
uint8_t data[];
};

Expand Down Expand Up @@ -260,6 +260,7 @@ struct ofi_mr_cache {

struct ofi_mr_storage storage;
struct dlist_entry lru_list;
struct dlist_entry flush_list;
pthread_mutex_t lock;

size_t cached_cnt;
Expand Down
41 changes: 21 additions & 20 deletions prov/util/src/util_mr_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ static void util_mr_uncache_entry(struct ofi_mr_cache *cache,
util_mr_uncache_entry_storage(cache, entry);

if (entry->use_cnt == 0) {
dlist_remove_init(&entry->lru_entry);
util_mr_free_entry(cache, entry);
dlist_remove(&entry->list_entry);
dlist_insert_tail(&entry->list_entry, &cache->flush_list);
} else {
cache->uncached_cnt++;
cache->uncached_size += entry->info.iov.iov_len;
Expand All @@ -155,15 +155,26 @@ bool ofi_mr_cache_flush(struct ofi_mr_cache *cache)
struct ofi_mr_entry *entry;

pthread_mutex_lock(&cache->monitor->lock);
while (!dlist_empty(&cache->flush_list)) {
dlist_pop_front(&cache->flush_list, struct ofi_mr_entry,
entry, list_entry);
FI_DBG(cache->domain->prov, FI_LOG_MR, "flush %p (len: %zu)\n",
entry->info.iov.iov_base, entry->info.iov.iov_len);
pthread_mutex_unlock(&cache->monitor->lock);

util_mr_free_entry(cache, entry);
pthread_mutex_lock(&cache->monitor->lock);
}

if (dlist_empty(&cache->lru_list)) {
pthread_mutex_unlock(&cache->monitor->lock);
return false;
}

do {
dlist_pop_front(&cache->lru_list, struct ofi_mr_entry,
entry, lru_entry);
dlist_init(&entry->lru_entry);
entry, list_entry);
dlist_init(&entry->list_entry);
FI_DBG(cache->domain->prov, FI_LOG_MR, "flush %p (len: %zu)\n",
entry->info.iov.iov_base, entry->info.iov.iov_len);

Expand Down Expand Up @@ -197,7 +208,7 @@ void ofi_mr_cache_delete(struct ofi_mr_cache *cache, struct ofi_mr_entry *entry)
util_mr_free_entry(cache, entry);
return;
}
dlist_insert_tail(&entry->lru_entry, &cache->lru_list);
dlist_insert_tail(&entry->list_entry, &cache->lru_list);
}
pthread_mutex_unlock(&cache->monitor->lock);
}
Expand Down Expand Up @@ -333,7 +344,7 @@ int ofi_mr_cache_search(struct ofi_mr_cache *cache, const struct fi_mr_attr *att

cache->hit_cnt++;
if ((*entry)->use_cnt++ == 0)
dlist_remove_init(&(*entry)->lru_entry);
dlist_remove_init(&(*entry)->list_entry);

unlock:
pthread_mutex_unlock(&cache->monitor->lock);
Expand Down Expand Up @@ -366,7 +377,7 @@ struct ofi_mr_entry *ofi_mr_cache_find(struct ofi_mr_cache *cache,

cache->hit_cnt++;
if ((entry)->use_cnt++ == 0)
dlist_remove_init(&(entry)->lru_entry);
dlist_remove_init(&(entry)->list_entry);

unlock:
pthread_mutex_unlock(&cache->monitor->lock);
Expand Down Expand Up @@ -423,19 +434,8 @@ void ofi_mr_cache_cleanup(struct ofi_mr_cache *cache)
cache->search_cnt, cache->delete_cnt, cache->hit_cnt,
cache->notify_cnt);

pthread_mutex_lock(&cache->monitor->lock);
while (!dlist_empty(&cache->lru_list)) {
dlist_pop_front(&cache->lru_list, struct ofi_mr_entry,
entry, lru_entry);
assert(entry->use_cnt == 0);
dlist_init(&entry->lru_entry);
util_mr_uncache_entry_storage(cache, entry);
pthread_mutex_unlock(&cache->monitor->lock);

util_mr_free_entry(cache, entry);
pthread_mutex_lock(&cache->monitor->lock);
}
pthread_mutex_unlock(&cache->monitor->lock);
while (ofi_mr_cache_flush(cache))
;

pthread_mutex_destroy(&cache->lock);
ofi_monitor_del_cache(cache);
Expand Down Expand Up @@ -548,6 +548,7 @@ int ofi_mr_cache_init(struct util_domain *domain,

pthread_mutex_init(&cache->lock, NULL);
dlist_init(&cache->lru_list);
dlist_init(&cache->flush_list);
cache->cached_cnt = 0;
cache->cached_size = 0;
cache->uncached_cnt = 0;
Expand Down

0 comments on commit 3d01df7

Please sign in to comment.