On Thu, Apr 6, 2017 at 7:57 PM, Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, Apr 06, 2017 at 05:13:52PM +0300, Matan Barak wrote: >> On Thu, Apr 6, 2017 at 12:05 AM, Hefty, Sean <sean.hefty@xxxxxxxxx> wrote: >> > Mostly questions. >> > >> >> @@ -1628,7 +1148,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 = idr_read_cq(cmd.cq_handle, file->ucontext, 0); >> >> + cq = uobj_get_obj_read(cq, cmd.cq_handle, file->ucontext); >> > >> > (I noticed rereg_mr used a write lock.) >> > Is a read lock sufficient here? >> > >> >> I guess we don't want to allow two concurrent resize_cq, so write >> lock should be better here. However, I don't want to change logic in >> these patches. I think such a patch is better of being separate with >> a proper commit message. We don't step on these bugs, as there's no >> lock at the verbs layer. So usually, device drivers provide the >> locking their themselves. > > It makes sense to split the patch, but the 'write lock' nee > 'exclusive' seems like the wrong approach, we need an actual mutex > just like for multicast. > Wjy? Let's look at that from the application point of view. When two threads try to resize the cq concurrently, what is the CQ actual size? It could be any one the request sizes. More than that, it doesn't really make sense to resize the same CQ concurrently. I think it's safe to return -EBUSY to one of these system calls. > Jason Matan -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
