On Fri, Jan 20, 2017 at 11:38 AM, Tejun Heo <tj@xxxxxxxxxx> wrote: > Hello, Gwendal. > > On Fri, Jan 20, 2017 at 02:19:46PM -0500, Tejun Heo wrote: >> On Fri, Jan 20, 2017 at 12:08:10PM -0500, tedheadster wrote: >> > kobject_get(pata_legacy.1): ref=3++ (ata_tport_add:ata_host_register:ata_host_activate) >> > kobject_get(pata_legacy.1): ref=4++ (ata_tport_add:ata_host_register:ata_host_activate) >> > XXX kobject_get(pata_legacy.1): ref=5++ (kobject_add:device_add:ata_tport_add) >> > XXX kobject_put(pata_legacy.1): ref=6-- (device_del:ata_tport_delete:ata_host_detach) >> > XXX kobject_put(pata_legacy.1): ref=5-- (ata_tport_delete:ata_host_detach:legacy_init [pata_legacy]) >> > XXX kobject_put(pata_legacy.1): ref=4-- (klist_put:klist_del:device_del) >> > XXX kobject_put(pata_legacy.1): ref=3-- (klist_devices_put:klist_put:klist_del) >> > XXX kobject_put(pata_legacy.1): ref=2-- (platform_device_unregister:legacy_init [pata_legacy]:do_one_initcall) >> >> So, three gets from the transport path but only two puts. That seems >> to be the culprit. Looking into it. > > So, Matthew discovered that pata_legacy isn't releasing resources > after probe failure. After some debugging, it turns out that the > transport code is taking three refs but only putting two preventing > the ata port from going away. > > Can you please explain why the libata transport code is explicitly > pinning the parent device and then putting it in the release methods? > device_add/del() already gets and puts the parent, so I don't get why > this part is necessary. Is this intentional? It was intentional. When I wrote the code, I used other transport as inspiration, in particular drivers/scsi/scsi_transport_sas.c. For instance, in sas_port_alloc, "port->dev.parent = get_device(parent);", undo in sas_port_release(). >From your debug code, in case of ATA at least, it looks unnecessary. Moreover, in the error path of ata_XXX_add, a put_device(&link->tdev) Also,How ata_tport_add() increase the reference twice before calling device_add()? > > Also, it looks like there is a ref leak on the transport device > itself. Its release function never gets called and thus the parent > device (ata_port) stays pinned too. That's the root cause of pata port not released. Can we run your debug code one more time, enabling ap->tdev.kobj.release_debug? Thanks, Gwendal. > > Thanks. > > -- > tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
