| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1783906.XctYjeUNLr@vostro.rjw.lan>
Date: Fri, 29 Jan 2016 22:35:18 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Tomeu Vizoso <tomeu.vizoso@...labora.com>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
Alan Stern <stern@...land.harvard.edu>,
martyn.welch@...labora.co.uk, Ulf Hansson <ulf.hansson@...aro.org>,
Daniel Kurtz <djkurtz@...omium.org>,
David Airlie <airlied@...ux.ie>,
Tomas Winkler <tomas.winkler@...el.com>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
dri-devel@...ts.freedesktop.org,
Russell King <linux@....linux.org.uk>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Kevin Hilman <khilman@...nel.org>,
Tony Lindgren <tony@...mide.com>, Len Brown <lenb@...nel.org>,
Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
Pavel Machek <pavel@....cz>,
linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v12 2/4] PM / Domains: add setter for dev.pm_domain
On Friday, January 29, 2016 06:35:49 PM Andy Shevchenko wrote:
> On Fri, Jan 29, 2016 at 5:51 PM, Andy Shevchenko
> <andy.shevchenko@...il.com> wrote:
> > On Fri, Jan 29, 2016 at 5:38 PM, Andy Shevchenko
> > <andy.shevchenko@...il.com> wrote:
> >> On Thu, Jan 7, 2016 at 5:46 PM, Tomeu Vizoso <tomeu.vizoso@...labora.com> wrote:
> >>> Adds a function that sets the pointer to dev_pm_domain in struct device
> >>> and that warns if the device has already finished probing. The reason
> >>> why we want to enforce that is because in the general case that can
> >>> cause problems and also that we can simplify code quite a bit if we can
> >>> always assume that.
> >>>
> >>> This patch also changes all current code that directly sets the
> >>> dev.pm_domain pointer.
> >>
> >>
> >>> --- a/drivers/acpi/acpi_lpss.c
> >>> +++ b/drivers/acpi/acpi_lpss.c
> >>
> >>> @@ -875,13 +876,14 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
> >>>
> >>> switch (action) {
> >>> case BUS_NOTIFY_BIND_DRIVER:
> >>> - pdev->dev.pm_domain = &acpi_lpss_pm_domain;
> >>> + dev_pm_domain_set(&pdev->dev, &acpi_lpss_pm_domain);
> >>> break;
> >>> case BUS_NOTIFY_DRIVER_NOT_BOUND:
> >>> case BUS_NOTIFY_UNBOUND_DRIVER:
> >>> pdev->dev.pm_domain = NULL;
> >>
> >> Missed?
> >>
> >>> break;
> >>> case BUS_NOTIFY_ADD_DEVICE:
> >>> + dev_pm_domain_set(&pdev->dev, &acpi_lpss_pm_domain);
> >>> if (pdata->dev_desc->flags & LPSS_LTR)
> >>> return sysfs_create_group(&pdev->dev.kobj,
> >>> &lpss_attr_group);
> >>> @@ -889,6 +891,7 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
> >>> case BUS_NOTIFY_DEL_DEVICE:
> >>> if (pdata->dev_desc->flags & LPSS_LTR)
> >>> sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
> >>> + dev_pm_domain_set(&pdev->dev, NULL);
> >>> break;
> >>> default:
> >>> break;
> >>
> >> This looks wrong. I didn't test yet, but I have concerns here. Why did
> >> you add those calls?
> >
> >
> > Okay, for first glance it seems working on Intel Braswell.
>
> One more test unveils this one
>
> # modprobe -r sdhci-acpi
> [ 1289.909441] ------------[ cut here ]------------
> [ 1289.918205] WARNING: CPU: 1 PID: 4374 at
> /home/andy/prj/linux-otc/drivers/base/power/common.c:150
> dev_pm_domain_set+0x51/0x60()
> [ 1289.934681] PM domains can only be changed for unbound devices
> [ 1289.944843] Modules linked in: sdhci_acpi(-) sdhci mmc_core
> led_class [last unloaded: dw_dmac_core]
> [ 1289.958802] CPU: 1 PID: 4374 Comm: modprobe Not tainted 4.5.0-rc1+ #3
> [ 1289.969736] ffffffff81c38330 ffff88007bb53d18 ffffffff8133162f
> ffff88007bb53d60
> [ 1289.981844] ffff88007bb53d50 ffffffff8105cd12 ffff88017a007410
> 0000000000000000
> [ 1289.993996] 0000000000000001 0000000000000080 0000000000000000
> ffff88007bb53db0
> [ 1290.006123] Call Trace:
> [ 1290.012600] [<ffffffff8133162f>] dump_stack+0x44/0x55
> [ 1290.022052] [<ffffffff8105cd12>] warn_slowpath_common+0x82/0xc0
> [ 1290.032462] [<ffffffff8105cd9c>] warn_slowpath_fmt+0x4c/0x50
> [ 1290.042589] [<ffffffff814867c1>] dev_pm_domain_set+0x51/0x60
> [ 1290.052695] [<ffffffff813a62a9>] acpi_dev_pm_detach+0x3f/0x84
> [ 1290.062910] [<ffffffff81486747>] dev_pm_domain_detach+0x27/0x30
> [ 1290.073294] [<ffffffff81480188>] platform_drv_remove+0x38/0x40
> [ 1290.083585] [<ffffffff8147da51>] __device_release_driver+0xa1/0x160
> [ 1290.094335] [<ffffffff8147e5a6>] driver_detach+0xa6/0xb0
> [ 1290.104009] [<ffffffff8147d435>] bus_remove_driver+0x55/0xd0
> [ 1290.114053] [<ffffffff8147eccc>] driver_unregister+0x2c/0x50
> [ 1290.124076] [<ffffffff81480232>] platform_driver_unregister+0x12/0x20
> [ 1290.134957] [<ffffffffa003681c>] sdhci_acpi_driver_exit+0x10/0x12
> [sdhci_acpi]
> [ 1290.146730] [<ffffffff810ce0e9>] SyS_delete_module+0x199/0x240
> [ 1290.156907] [<ffffffff8100219b>] ? exit_to_usermode_loop+0x8b/0x90
> [ 1290.167478] [<ffffffff81002b83>] do_syscall_32_irqs_off+0x53/0xa0
> [ 1290.177949] [<ffffffff818b7f9a>] entry_INT80_compat+0x2a/0x40
> [ 1290.187986] ---[ end trace a8351c0506ee239b ]---
> [ 1290.213855] mmc0: card 0001 removed
>
> Which function is guilty?
acpi_dev_pm_detach() calls dev_pm_domain_set() to clear the pm_domain pointer.
One might argue that this is happening too early.
Thanks,
Rafael
Powered by blists - more mailing lists