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

[Rajat Jain]

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/