On Tue, Nov 14, 2017 at 12:29:14PM +0200, Yishai Hadas wrote: > It's not enough, there might be cases that protection_domain is NULL but > still the object is a parent domain as of in the CQ case where td may be > applicable but not the protection_domain. Better having here clear > indication which is set upon the creation API rather than some semantic. Ugh, the idea of a ibv_pd without a ibv_pd is a ugly. That is a whole bunch more checking and failure scenarios. I think I'd rather have the CQ receive a PD it doesn't need than do that.. > >>+struct ibv_pd *mlx5_alloc_parent_domain(struct ibv_context *context, > >>+ struct ibv_parent_domain_init_attr *attr) > >>+ pd->protection_domain= attr->pd; > > > >Don't we need some kind of ref counting here? > > > > We thought about it as well, however reference counting can't prevent race > conditions when more than one thread uses the API. Not really concerned about races. More concerned about helping the user use it properly.. > Similar behavior exists also in current code which puts the responsibility > on the application to correctly use the API instead of using some reference > counting /locks inside the libraries. We have refcounts in the kernel side for many things and generate errors if the user does stuff in the wrong order. > >What is the intention for the final version of this patch? > > The next patch of this RFC around QP creation introduces the > expected usage. Well, this transformation would have to ultimately be done in this patch or before. Looking more, this isn't good enough, at the very least I think you need to memcpy the struct ibv_pd portion so context and handle are set, as those are public ABIs. Jason -- 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
