Message ID | 20170208194330.GB25826@htj.duckdns.org |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Wed, Feb 8, 2017 at 11:43 AM, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Tue, Feb 07, 2017 at 12:21:37PM -0800, Gwendal Grignou wrote: >> I am wondering if we don't have a circular dependency: >> We do the final put_device (in scsi_host_put) on ap->scsi_host in >> ata_host_release(), but it is not called because >> [scsi_host]->shost_gendev.parent is &ap->tdev which hold the put on >> its parent, ap. >> >> If my understanding is correct, as Tejun pointed out, removing the put >> on ap in ata_tport_release and the get_device(parent) in ata_tport_add >> should unlock the situation. > > Heh, I'm not quite sure I follow but something like the following, right? That's correct. > > Matthew, can you please give this a try? > > Thanks. > > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index 7ef16c0..20e2b7a 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -224,7 +224,6 @@ static DECLARE_TRANSPORT_CLASS(ata_port_class, > > static void ata_tport_release(struct device *dev) > { > - put_device(dev->parent); > } > > /** > @@ -284,7 +283,7 @@ int ata_tport_add(struct device *parent, > device_initialize(dev); > dev->type = &ata_port_type; > > - dev->parent = get_device(parent); > + dev->parent = parent; > dev->release = ata_tport_release; > dev_set_name(dev, "ata%d", ap->print_id); > transport_setup_device(dev); > @@ -348,7 +347,6 @@ static DECLARE_TRANSPORT_CLASS(ata_link_class, > > static void ata_tlink_release(struct device *dev) > { > - put_device(dev->parent); > } > > /** > @@ -410,7 +408,7 @@ int ata_tlink_add(struct ata_link *link) > int error; > > device_initialize(dev); > - dev->parent = get_device(&ap->tdev); > + dev->parent = &ap->tdev; > dev->release = ata_tlink_release; > if (ata_is_host_link(link)) > dev_set_name(dev, "link%d", ap->print_id); > @@ -589,7 +587,6 @@ static DECLARE_TRANSPORT_CLASS(ata_dev_class, > > static void ata_tdev_release(struct device *dev) > { > - put_device(dev->parent); > } > > /** > @@ -662,7 +659,7 @@ static int ata_tdev_add(struct ata_device *ata_dev) > int error; > > device_initialize(dev); > - dev->parent = get_device(&link->tdev); > + dev->parent = &link->tdev; > dev->release = ata_tdev_release; > if (ata_is_host_link(link)) > dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno); -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I just tested this patch and it is working for me. - Matthew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 09, 2017 at 07:36:59PM -0500, tedheadster wrote:
> I just tested this patch and it is working for me.
Awesome, Gwendal, care to write up the path description?
Thanks.
Gwendal, could you submit a formal patch so we can get it in the next kernel? - Matthew -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index 7ef16c0..20e2b7a 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -224,7 +224,6 @@ static DECLARE_TRANSPORT_CLASS(ata_port_class, static void ata_tport_release(struct device *dev) { - put_device(dev->parent); } /** @@ -284,7 +283,7 @@ int ata_tport_add(struct device *parent, device_initialize(dev); dev->type = &ata_port_type; - dev->parent = get_device(parent); + dev->parent = parent; dev->release = ata_tport_release; dev_set_name(dev, "ata%d", ap->print_id); transport_setup_device(dev); @@ -348,7 +347,6 @@ static DECLARE_TRANSPORT_CLASS(ata_link_class, static void ata_tlink_release(struct device *dev) { - put_device(dev->parent); } /** @@ -410,7 +408,7 @@ int ata_tlink_add(struct ata_link *link) int error; device_initialize(dev); - dev->parent = get_device(&ap->tdev); + dev->parent = &ap->tdev; dev->release = ata_tlink_release; if (ata_is_host_link(link)) dev_set_name(dev, "link%d", ap->print_id); @@ -589,7 +587,6 @@ static DECLARE_TRANSPORT_CLASS(ata_dev_class, static void ata_tdev_release(struct device *dev) { - put_device(dev->parent); } /** @@ -662,7 +659,7 @@ static int ata_tdev_add(struct ata_device *ata_dev) int error; device_initialize(dev); - dev->parent = get_device(&link->tdev); + dev->parent = &link->tdev; dev->release = ata_tdev_release; if (ata_is_host_link(link)) dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);