| Message ID | 20201005142109.977461657@linuxfoundation.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Hi! > From: Yu Kuai <yukuai3@huawei.com> > > [ Upstream commit 1a26044954a6d1f4d375d5e62392446af663be7a ] > > if of_find_device_by_node() succeed, exynos_iommu_of_xlate() doesn't have > a corresponding put_device(). Thus add put_device() to fix the exception > handling for this function implementation. Okay, this looks reasonable, but... Do we miss put_device() in normal path, too? I'd expect another put_device at end of exynos_iommu_of_xlate() or perhaps in release path somewhere... Best regards, Pavel
Hi Pavel, On 07.10.2020 11:47, Pavel Machek wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> [ Upstream commit 1a26044954a6d1f4d375d5e62392446af663be7a ] >> >> if of_find_device_by_node() succeed, exynos_iommu_of_xlate() doesn't have >> a corresponding put_device(). Thus add put_device() to fix the exception >> handling for this function implementation. > Okay, this looks reasonable, but... > > Do we miss put_device() in normal path, too? I'd expect another > put_device at end of exynos_iommu_of_xlate() or perhaps in release > path somewhere... Frankly, there is no release path, so there is no need for put_device. Once initialized, Exynos IOMMU stays in the system forever. There is no point to remove IOMMU nor the API for that. Keeping increased refcount for its device just matches this behavior. If the missing put_device() is really a problem, then we can move it from the error path just after data = platform_get_drvdata(sysmmu) assignment. Feel free to send a patch if you think this is a more appropriate approach. Best regards
Hi! > >> From: Yu Kuai <yukuai3@huawei.com> > >> > >> [ Upstream commit 1a26044954a6d1f4d375d5e62392446af663be7a ] > >> > >> if of_find_device_by_node() succeed, exynos_iommu_of_xlate() doesn't have > >> a corresponding put_device(). Thus add put_device() to fix the exception > >> handling for this function implementation. > > Okay, this looks reasonable, but... > > > > Do we miss put_device() in normal path, too? I'd expect another > > put_device at end of exynos_iommu_of_xlate() or perhaps in release > > path somewhere... > > Frankly, there is no release path, so there is no need for put_device. > Once initialized, Exynos IOMMU stays in the system forever. There is no > point to remove IOMMU nor the API for that. Keeping increased refcount > for its device just matches this behavior. > > If the missing put_device() is really a problem, then we can move it > from the error path just after data = platform_get_drvdata(sysmmu) > assignment. Feel free to send a patch if you think this is a more > appropriate approach. exynos_iommu_detach_device() looks like a place where resources could be freed? But if there's no release path, we don't really need to do anything. Sorry about the noise. Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 1bd0cd7168dfc..4bf6049dd2c79 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1302,13 +1302,17 @@ static int exynos_iommu_of_xlate(struct device *dev, return -ENODEV; data = platform_get_drvdata(sysmmu); - if (!data) + if (!data) { + put_device(&sysmmu->dev); return -ENODEV; + } if (!owner) { owner = kzalloc(sizeof(*owner), GFP_KERNEL); - if (!owner) + if (!owner) { + put_device(&sysmmu->dev); return -ENOMEM; + } INIT_LIST_HEAD(&owner->controllers); mutex_init(&owner->rpm_lock);