@@ -1165,7 +1165,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
(unsigned long) cmd.response + sizeof resp,
in_len - sizeof cmd, out_len - sizeof resp);
- cq = uobj_get_obj_read(cq, cmd.cq_handle, file->ucontext);
+ cq = uobj_get_obj_write(cq, cmd.cq_handle, file->ucontext);
if (!cq)
return -EINVAL;
@@ -1180,7 +1180,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
ret = -EFAULT;
out:
- uobj_put_obj_read(cq);
+ uobj_put_obj_write(cq);
return ret ? ret : in_len;
}
@@ -1914,7 +1914,7 @@ static int modify_qp(struct ib_uverbs_file *file,
if (!attr)
return -ENOMEM;
- qp = uobj_get_obj_read(qp, cmd->base.qp_handle, file->ucontext);
+ qp = uobj_get_obj_write(qp, cmd->base.qp_handle, file->ucontext);
if (!qp) {
ret = -EINVAL;
goto out;
@@ -1986,7 +1986,7 @@ static int modify_qp(struct ib_uverbs_file *file,
}
release_qp:
- uobj_put_obj_read(qp);
+ uobj_put_obj_write(qp);
out:
kfree(attr);
@@ -3607,7 +3607,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
INIT_UDATA(&udata, buf + sizeof cmd, NULL, in_len - sizeof cmd,
out_len);
- srq = uobj_get_obj_read(srq, cmd.srq_handle, file->ucontext);
+ srq = uobj_get_obj_write(srq, cmd.srq_handle, file->ucontext);
if (!srq)
return -EINVAL;
@@ -3616,7 +3616,7 @@ ssize_t ib_uverbs_modify_srq(struct ib_uverbs_file *file,
ret = srq->device->modify_srq(srq, &attr, cmd.attr_mask, &udata);
- uobj_put_obj_read(srq);
+ uobj_put_obj_write(srq);
return ret ? ret : in_len;
}
@@ -73,6 +73,15 @@ static inline struct ib_uobject *__uobj_get(const struct uverbs_obj_type *type,
#define uobj_get_write(_type, _id, _ucontext) \
__uobj_get(&(_type), true, _ucontext, _id)
+#define uobj_get_obj_write(_type, _id, _ucontext) \
+({ \
+ struct ib_uobject *uobj = \
+ __uobj_get(&uobj_get_type(_type), \
+ true, _ucontext, _id); \
+ \
+ (struct ib_##_type *)(IS_ERR(uobj) ? NULL : uobj->object); \
+})
+
static inline void uobj_put_read(struct ib_uobject *uobj)
{
rdma_lookup_put_uobject(uobj, false);
@@ -86,6 +95,9 @@ static inline void uobj_put_write(struct ib_uobject *uobj)
rdma_lookup_put_uobject(uobj, true);
}
+#define uobj_put_obj_write(_obj) \
+ uobj_put_write((_obj)->uobject)
+
static inline int __must_check uobj_remove_commit(struct ib_uobject *uobj)
{
return rdma_remove_commit_uobject(uobj);
Verbs handlers such as resize_cq, modify_qp and modify_srq should use an exclusive lock in order to prevent concurrent modification to the same object. This usually makes sense, as modify the same QP concurrently to state X and state Y (where X != Y) could either lead to the QP begin in its original state, in state X or in state Y. The user could either get a success or an error return code for each call. This goes similarly for resize_cq and modify_srq. However, sometimes this behavior is valid. For example, changing two different QP/SRQ attributes in the same state is a valid behaviour for two concurrent modify_qp/modify_srq. The same goes for resizing a CQ concurrently to the same size. These seems to be esoteric cases, but it does changes the current user's experience. This could be solved differently by using a mutex and serialize these calls in the uverbs handlers. However, I'm not sure these bizarre usages qualify introducing more locks. Signed-off-by: Matan Barak <matanb@mellanox.com> --- Hi Doug, This RFC's goal is to handle the abnormal behavior of the current uverbs_cmd modify_xxxx handler, where the object is modified, but only a read lock is acquired. The current behaviour is inconsistent and racy in respect of user-space applications. It could be solved by (at least) two approaches. This patch demonstrates the simplified one. Since it could lead to changes in how applications behave, I thought it's better to first post it as a RFC. Regards, Matan drivers/infiniband/core/uverbs_cmd.c | 12 ++++++------ include/rdma/uverbs_std_types.h | 12 ++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-)