Re: [PATCH 00/14] iommu: fix device leaks

[Robin Murphy]

On 2025-09-30 7:21 pm, Jason Gunthorpe wrote:
> On Thu, Sep 25, 2025 at 02:27:42PM +0200, Johan Hovold wrote:
> > This series fixes device leaks in the iommu drivers, which pretty
> > consistently failed to drop the reference taken by
> > of_find_device_by_node() when looking up iommu platform devices.
>
> Yes, they are mis-designed in many ways :\

Historically they weren't really leaks either, just spare references on 
a device which at that point could definitely never go away. I suppose 
now that we finally have the .of_xlate calls in IOMMU registration, it 
would be possible for some error during probe to cause the IOMMU driver 
to fail to bind, at which point the references could rightly be 
considered leaked, however these are all embedded platforms with 
essentially zero chance of platform device hotplug, especially not of 
IOMMU devices, so realistically those references still don't matter to 
anything other than code checkers.

In summary; meh.
> IDK if it is worth fixing like this, or if more effort should be put
> to make the drivers use of_xlate properly - the arm smmu drivers show
> the only way to use it..

The SMMU drivers are really doing the same thing, they just defer that 
lookup operation to .probe_device time (largely for historical reasons, 
I think), and they use bus_find_device_by_node() rather than the 
specific of_ version since they support ACPI too. And they do happen to 
include the put_device(), since apparently I was paying full attention 
that day.

> But if staying like this then maybe add a little helper?
>
> void *iommu_xlate_to_iommu_drvdata(const struct of_phandle_args *args);
>
> Put the whole racy of_find_device_by_node / put_device /
> platform_get_drvdata sequence is in one tidy function.. With
> documentation it is not safe don't use it in new code?

It's not racy; if we're calling the .of_xlate op (or really any op for 
that matter), it's because an IOMMU driver has registered (or is 
registering) for the given fwnode, which means it is bound to the 
corresponding struct device. Thus as above, in that context said struct 
device, nor said IOMMU driver's drvdata, ain't goin' nowhere.

Thanks,
Robin.