On 2021/5/27 21:16, Jason Gunthorpe wrote: > On Thu, May 27, 2021 at 01:08:37PM +0000, Marciniszyn, Mike wrote: >>> +++ b/drivers/infiniband/hw/hfi1/verbs.c >>> @@ -534,7 +534,8 @@ static inline void hfi1_handle_packet(struct >>> hfi1_packet *packet, >>> * Notify rvt_multicast_detach() if it is waiting for us >>> * to finish. >>> */ >>> - if (atomic_dec_return(&mcast->refcount) <= 1) >>> + refcount_dec(&mcast->refcount); >>> + if (refcount_read(&mcast->refcount) <= 1) >>> wake_up(&mcast->wait); >> >> Is there refcount_ that preserves the atomic characteristics of the single call? > > You are supposed to us refcount_dec_and_test() for patterns like this, > this hunk looks wrong to me > > Jason > I made a mistake, thank you. refcount_dec_and_test() test only if the refcount is 0, so I couldn't find a good way to replace the atomic_t with refcount_t here. And there is no refcount_dec/inc_return() for a refcount_t, I found some previous disscussdion: https://www.openwall.com/lists/kernel-hardening/2016/11/28/8 https://www.openwall.com/lists/kernel-hardening/2016/12/01/1 https://lkml.org/lkml/2017/9/1/389 https://yhbt.net/lore/all/20200921112218.GB2139@willie-the-truck/t/#md37f8b6084bccbc0ebf76ba249c069372b4d8497 I think such functions undermines the design of refcount. But to be honest, I didn't find a clear reason. Peter, could you please explain why you said "add_return and sub_return are horrible interface for refcount"? You can review the whole patch at: https://patchwork.kernel.org/project/linux-rdma/patch/1621925504-33019-14-git-send-email-liweihang@xxxxxxxxxx/ Thanks Weihang
