On Wed, May 27, 2015 at 11:47:31PM +0300, Yishai Hadas wrote:
> That's correct, it was chosen from performance reasons to enable parallel
> commands as part of ib_uverbs_write with minimum synchronization overhead
> comparing the rwsem.
The locking scheme makes no sense, see my other email, design it for a
rwsem and then consider rcu if necessary.
Hint: Get rid of dev->disassociated and use ib_dev == null to
accomplish the same thing. Now it is clear that ib_dev is what must be
protected by a rwlock and the locking scheme will flow sanely from
that point. If you must use RCU for performance then it is now clear
that rcu_dereference must be applied to the ib_dev.
Then the other locking can stop being so subtle:
-----
ib_uverbs_close:
down(lists_mutex)
tmp = file->ucontext;
if (tmp) {
list_del(&file->list);
file->ucontext = NULL;
}
up(lists_mutex);
if (tmp)
ib_uverbs_cleanup_ucontext(file, tmp);
-----
ib_uverbs_free_hw_resources:
down(lists_mutex)
while (!lists_empty(&uverbs_dev->uverbs_file_list) {
file = list_first_entry(&uverbs_dev->uverbs_file_list, list)
tmp = file->ucontext;
list_del(&file->list);
file->ucontext = NULL;
kref_get(&file->ref);
up(list_mutex);
ib_uverbs_cleanup_ucontext(file, tmp);
down(list_mutex);
kref_put(&file->ref,ib_uverbs_release_file);
}
up(list_mutex);
Now the krefs are unquestionably paired, we don't leave a dangling
ucontext pointer, we don't leave a dangling list entry, locking of the
list is explicit and doesn't rely on an unlocked access with an
implicit exclusion. Basically, it is actually obvious what each lock
is protecting.
> In case the low level driver can be unloaded (e.g. rmmod mlx4_ib)
> unconditionally to active uverbs clients we should prevent the module
> dependency by ignoring the get/put mechanism. Hope that it clarifies the
> usage here.
Okay, that does make sense. But I do suspect try_module_get should still
be run, just immediately released. Otherwise the check for in-progress
module removal is skipped.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
