> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, March 23, 2023 10:31 PM
>
> 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)
My bad.
>
> > > + /* 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".
>
> ??
>
It's not intuitive when moving the refcnt why we subtract num_devices
from the old_hwpt only when it's greater than 1. It's really about
the destroy must be done by the caller.
