On Thu, Mar 23, 2023 at 07:31:02AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > Sent: Wednesday, March 22, 2023 3:15 AM > > + > > + mutex_lock(&idev->igroup->lock); > > + > > + if (igroup->hwpt == NULL) { > > + rc = -EINVAL; > > + goto err_unlock; > > + } > > + > > + if (hwpt == igroup->hwpt) { > > + mutex_unlock(&idev->igroup->lock); > > + return NULL; > > + } > > goto err_unlock; No, this is a success path, it should not jumpt to an err label or use return ERR_PTR(0) > > + /* Move the refcounts held by the device_list to the new hwpt */ > > + refcount_add(num_devices, &hwpt->obj.users); > > + if (num_devices > 1) > > + WARN_ON(refcount_sub_and_test(num_devices - 1, > > + &old_hwpt->obj.users)); > > A comment is welcomed to match "caller must destroy old_hwpt". ?? Jason