diff mbox

pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems

Message ID 20161205192310.GA30331@mtj.duckdns.org
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Tejun Heo Dec. 5, 2016, 7:23 p.m. UTC
Hello,

Hmm... I'm a bit confused.  These resources are tied to the platform
device which is unregistered on probe failure which will invoke
devres_release_all().

Matthew, can you please apply the following patch and see whether
device_release() gets invoked?

Thanks.


--
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

Comments

Tejun Heo Dec. 12, 2016, 5:01 p.m. UTC | #1
Hello,

On Fri, Dec 09, 2016 at 12:57:02PM -0500, tedheadster wrote:
> ​Tejun,
>   here is the patch I applied:
> 
> diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
> index 4fe9d21..5c6c578 100644
> --- a/drivers/ata/pata_legacy.c
> +++ b/drivers/ata/pata_legacy.c
> @@ -963,6 +963,9 @@ static __init int legacy_init_one(struct legacy_probe
> *probe)
>         if (IS_ERR(pdev))
>                 return PTR_ERR(pdev);
> 
> +       if (!devres_open_group(&pdev->dev, legacy_init_one, GFP_KERNEL))
> +               return -ENOMEM;

Can you please drop the explicit group open/remove/release?

>         ret = -EBUSY;
>         if (devm_request_region(&pdev->dev, io, 8, "pata_legacy") == NULL ||
>             devm_request_region(&pdev->dev, io + 0x0206, 1,
> @@ -1009,11 +1012,14 @@ static __init int legacy_init_one(struct
> legacy_probe *probe)
>                 if (!ata_dev_absent(dev)) {
>                         legacy_host[probe->slot] = host;
>                         ld->platform_dev = pdev;
> +                       devres_remove_group(&pdev->dev, legacy_init_one);
>                         return 0;
>                 }
>         }
>         ata_host_detach(host);
>  fail:
> +       devres_release_group(&pdev->dev, legacy_init_one);
> +       printk("XXX pata_legacy: unregistering platform dev %p\n", pdev);
>         platform_device_unregister(pdev);

So, the thing is that when the platform device is released here, it
should automatically trigger release of all resources attached to it
through...

>         return ret;
>  }
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index a235085..8e8948e 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -214,6 +214,7 @@ static void device_release(struct kobject *kobj)
>          * is deleted but alive, so release devres here to avoid
>          * possible memory leak.
>          */
> +       printk("XXX device_release: invoking devres_release_all\n");
>         devres_release_all(dev);

the devres_release_all() call here.

Can you please try to verify that devres_release_all() is being
invoked from platform device release?  I'll try to see if I can repro
the problem here.

thanks.
diff mbox

Patch

diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index bce2a8c..0dd72ce 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -1013,6 +1013,7 @@  static __init int legacy_init_one(struct legacy_probe *probe)
 	}
 	ata_host_detach(host);
 fail:
+	printk("XXX pata_legacy: unregistering platform dev %p\n", pdev);
 	platform_device_unregister(pdev);
 	return ret;
 }
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ce057a5..a3d112d 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -237,6 +237,7 @@  static void device_release(struct kobject *kobj)
 	 * is deleted but alive, so release devres here to avoid
 	 * possible memory leak.
 	 */
+	printk("XXX device_release: invoking devres_release_all\n");
 	devres_release_all(dev);
 
 	if (dev->release)