Re: [PATCH] kobject: Ensure child's resources get released before parent's resources

[Rajat Jain]

[Fixed the linux-pci mailing list address]

On Mon, Dec 14, 2015 at 2:33 PM, Rajat Jain <rajatxjain@xxxxxxxxx> wrote:
> On Mon, Dec 14, 2015 at 1:40 PM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>> On Mon, Dec 14, 2015 at 11:02:46AM -0800, Rajat Jain wrote:
>>> If the only remaining reference to a parent, is the one taken by
>>> the child (in kobject_add_internal()), then when the last
>>> reference to the child goes away, both child and its parents
>>> shall be released. However, currently the resources of parent
>>> get released first, followed by the child's resources:
>>>
>>> kobject_cleanup(child)
>>>     ....
>>>     kobject_del(child)
>>>         ....
>>>         kobject_put(child->parent) -> results in parent's release()
>>>         ...
>>>     child->kobj_type->release() -> Child's release()
>>>
>>> This is a problem because the child's release() method may still
>>> need to use parent resources or memory for its own cleanup. E.g.
>>> child may need parent pointer for dma_free_coherent() etc.
>>>
>>> Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx>
>>> Signed-off-by: Rajat Jain <rajatxjain@xxxxxxxxx>
>>
>> Why are you listed twice here?
>
> Ah, sorry, I'll remove that.
>
>>
>> Where in the kernel is the parent being freed before the child that is
>> causing this issue to happen?  We should fix that root cause first...
>
> Umm, are you saying that it is a bug to reach a scenario where all
> references to a parent, except the ones made by the child, are gone?
>
> Sorry, I should have given more context here. Here is the scenario
> where I came across this situation, and I'd appreciate any suggestions
> on how to better deal with this situation:
>
> I have 2 modules (random names here):
>
> user_interface.ko <--- pci_driver.ko
>
> 1) user_interface.ko
>      - exports some interfaces (char driver etc) to the userspace,
>      - allows low-level device drivers to register devices via some
> API (user_interface_add() / user_interface_del())
>      - Userspace can issue some transactions. Each transaction results
> in a child kobject being attached to the device's kobject.
>      - Low level drivers also provide a release() function that can
> get called AFTER user_interface_del() if there are transactions
> in-flight.
>      - Low level drivers should allow operation of the device until
> release() gets called.
>
> 2) Low level drivers such as pci_driver.ko:
>     - attach to the actual physical devices (PCI device in this case)
>     - create a custom device (that has an embedded "struct device")
> and register this new custom device with the user_interface.ko.
>     - also attaches a release() function to the device. This release()
> would get called when all references to the device are dropped.
>     - The entities holding the reference to the device are:
>           * 1 reference by the pci_driver.ko itself (when it did
> device_initialize())
>           * 1 reference by the user_interface.ko (During user_interface_add())
>           * 1 reference for each transaction in-flight (a child
> kobject under the device)
>
> 3) Now, we want to allow removing (rmmod) the low level driver pci_driver.ko.
>     - Before returning from PCI remove method, need to ensure that
> release() has been called.
>     - So we do call user_interface_del(dev) - drops the reference that
> user_interface.ko was holding.
>     - pci_driver.ko gives up its own reference so that release()
> method can get called.
>     - At this time, the device is just waiting for transactions
> in-flight to get completed i.e only the child kobjects hold the
> references.
>
> When the last transaction gets completed, I end up in the situation
> described in the patch commit log. I'd be very glad if you can provide
> suggestions on how to achieve this or if there is anything I am
> missing?
>
> On a side note, my poor understanding of the device model came out to
> that it does (or may be should) guarantee that all children are freed
> before the parent is freed. Is that not the case?
>
> Thanks,
>
> Rajat
>
>>
>> thanks,
>>
>> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/