On Fri, May 5, 2017 at 2:24 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, May 05, 2017 at 02:08:37PM -0400, Rob Clark wrote:
>> It looks like it *used* to make sense to free the device. But now it is
>> embedded in 'struct iommu' (which is allocated or embedded in something
>> that the device allocated).
>>
>> Spotted when testing qcom_iommu with CONFIG_DEBUG_TEST_DRIVER_REMOVE.
>>
>> Fixes: 39ab955 ("iommu: Add sysfs bindings for struct iommu_device")
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>> ---
>> drivers/iommu/iommu-sysfs.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/iommu/iommu-sysfs.c b/drivers/iommu/iommu-sysfs.c
>> index c58351e..ad19cbb 100644
>> --- a/drivers/iommu/iommu-sysfs.c
>> +++ b/drivers/iommu/iommu-sysfs.c
>> @@ -34,7 +34,6 @@ static const struct attribute_group *iommu_dev_groups[] = {
>>
>> static void iommu_release_device(struct device *dev)
>> {
>> - kfree(dev);
>> }
>
> As per the documentation in the kernel tree, I now get to make fun of
> you for doing such a crazh and foolish thing!
>
> Come on, don't do that, a release function _HAS_ to free the memory
> involved. If not, then it is really broken...
There are shenanigans going on.. so release isn't counterpoint to a
_probe() which allocates some memory. See iommu_device_sysfs_add().
So I'm not the one you get to make fun of ;-)
This the correct thing to do. Whether the way the extra fake device
embedded in something allocated in the iommu driver's probe (and
free'd it *it's* _release()) stuff for iommu sysfs stuff works is
bonkers or not, I'll let someone else decide.. it was like that when
I got here.
BR,
-R
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
