> > What? You can't combine one thread doing > > > > if (refcount_dec_and_test(&srq->refcnt)) > > wake_up(&srq->wait); > > > > With > > if (!refcount_dec_and_test(&srq->refcnt)) > > wait_event(srq->wait, !refcount_read(&srq->refcnt)); > > > > It makes no sense, how is that a refcount??? > > > > They can't *BOTH* refcount_dec_and_test! > > > > I can understand doing > > > > if (refcount_dec_and_test(&srq->refcnt)) > > wake_up(&srq->wait); > > > > and then > > > > wait_event(srq->wait, !refcount_read(&srq->refcnt)); > > > > That makes perfect sense. > > Perhaps I am mistaken in my understanding of refcounts here, but > what is wrong with this? If it's not that the refcount is set to > 1 on resource creation (which, by your earlier suggestion of > checking refcnt == 1, seems to be fine), and both callers (the > resource event handler and resource destroy call) need to make > sure the destroy sequence only happens once (when the refcount > reaches 0), then using refcount_dec_and_test seems right to me. I think the issue here is trying to fit an optimized approach that was using atomics into a refcount. I have certain expectations when I see something called 'refcount' that this scheme really doesn't follow anymore, and those expectations are pretty much matched by the protections inside refcount against going to 0 and so on. The algorithm works OK, but I'm not sure it is a 'refcount', more of a 'usecnt'.. On the other hand, having the refcount overflow protections in this path does seem useful. I did a random audit of refcount_dec_and_test existing users and didn't find any that use this pattern. What you've actually built here is a rwsem out of an atomic and a wait queue.. So, what is wrong with a rwsem? > What we're doing is effectively the same as how mlx5 handles > this (see drivers/net/ethernet/mellanox/mlx5/core/srq.c) I > suppose using refcount_t requires one to follow a particular > model of usage but I'm not sure how this isn't right. I didn't say it was wrong, I asked 'how is that a refcount??' 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
