Re: [PATCH 09/14] iommufd: Add iommufd_device_replace()

Baolu Lu
2023-02-28T01:59:32Z

On 2/27/23 9:58 PM, Jason Gunthorpe wrote:
> On Sun, Feb 26, 2023 at 11:01:59AM +0800, Baolu Lu wrote:
>> On 2/25/23 8:27 AM, Jason Gunthorpe wrote:
>>> @@ -437,25 +517,77 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
>>> struct iommufd_ioas *ioas =
>>> container_of(pt_obj, struct iommufd_ioas, obj);
>>> - rc = iommufd_device_auto_get_domain(idev, ioas, pt_id);
>>> - if (rc)
>>> + destroy_hwpt = iommufd_device_auto_get_domain(idev, ioas, pt_id,
>>> + do_attach);
>>> + if (IS_ERR(destroy_hwpt))
>>> goto out_put_pt_obj;
>>> break;
>>> }
>>> default:
>>> - rc = -EINVAL;
>>> + destroy_hwpt = ERR_PTR(-EINVAL);
>>> goto out_put_pt_obj;
>>> }
>>> + iommufd_put_object(pt_obj);
>>> - refcount_inc(&idev->obj.users);
>>> - rc = 0;
>>> + /* This destruction has to be after we unlock everything */
>>> + if (destroy_hwpt)
>> Should this be
>>
>> if (!IS_ERR_OR_NULL(destroy_hwpt))
>>
>> ?
> Never use IS_ERR_OR_NULL ..

Can you please elaborate a bit on this? I can still see a lot of use of
it in the tree.

>
> What am I missing? all the flows that could possibly have err_ptr here
> do goto_out_put_pt_obj ?

Oh yes! You are right. All err_ptr's have gone to an error handling
path.

Best regards,
baolu