[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-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
