Re: PM domain change on unbound devices warning on ipmi_si unload

[Corey Minyard <minyard@acm.org>]

On 01/27/2016 10:45 PM, Joe Lawrence wrote:
> Starting in 4.5-rc1, I noticed this warning on ipmi_si driver removal:
>
> % modprobe ipmi_si
> % rmmod ipmi_si

Yes, I had just noticed this yesterday and I was going to try to trace 
it down today.
I had assumed it was an IPMI driver problem, but after looking at it a 
bit, I don't think
it is.  I'll try to bisect to see what caused this, nothing obvious 
jumps out

-corey

>
> bus: 'platform': driver_probe_device: matched device IPI0001:00 with driver ipmi_si
> bus: 'platform': really_probe: probing driver ipmi_si with device IPI0001:00
> ipmi_si IPI0001:00: ipmi_si: probing via ACPI
> ipmi_si IPI0001:00: [io  0x0ca2-0x0ca3] regsize 1 spacing 1 irq 0
> ipmi_si: Adding ACPI-specified kcs state machine
> driver: 'ipmi_si': driver_bound: bound to device 'IPI0001:00'
> bus: 'platform': really_probe: bound device IPI0001:00 to driver ipmi_si
> IPMI System Interface driver.
> ipmi_si: probing via SMBIOS
> ipmi_si: SMBIOS: io 0xda2 regsize 1 spacing 1 irq 0
> ipmi_si: Adding SMBIOS-specified kcs state machine
> ipmi_si: Trying ACPI-specified kcs state machine at i/o address 0xca2, slave address 0x0, irq 0
> Registering platform device 'ipmi_bmc.0674.66'. Parent at platform
> driver: 'ipmi': driver_bound: bound to device 'ipmi_bmc.0674.66'
> ipmi_si IPI0001:00: Found new BMC (man_id: 0x000077, prod_id: 0x0674, dev_id: 0x42)
> ipmi_si IPI0001:00: IPMI kcs interface initialized
> ------------[ cut here ]------------
> WARNING: CPU: 39 PID: 3678 at drivers/base/power/common.c:150 dev_pm_domain_set+0x52/0x60()
> PM domains can only be changed for unbound devices
> [ ... snip ... ]
> CPU: 39 PID: 3678 Comm: rmmod Tainted: G           OE   4.5.0-rc1+ #57
> Hardware name: Stratus ftServer 6800/G7LYY, BIOS BIOS Version 8.1:61 09/10/2015
> 0000000000000000 000000003883b68d ffff8820334d7d30 ffffffff8132caf0
> ffff8820334d7d78 ffff8820334d7d68 ffffffff8107f1b6 ffff8810351eca00
> 0000000000000000 0000000000000001 0000000001c2a330 0000000001c29010
> Call Trace:
> [<ffffffff8132caf0>] dump_stack+0x44/0x64
> [<ffffffff8107f1b6>] warn_slowpath_common+0x86/0xc0
> [<ffffffff8107f24c>] warn_slowpath_fmt+0x5c/0x80
> [<ffffffff8145e152>] dev_pm_domain_set+0x52/0x60
> [<ffffffff813a38ae>] acpi_dev_pm_detach+0x3f/0x84
> [<ffffffff8145e0d7>] dev_pm_domain_detach+0x27/0x30
> [<ffffffff814575f8>] platform_drv_remove+0x38/0x40
> [<ffffffff814557ba>] __device_release_driver+0x9a/0x140
> [<ffffffff81455968>] driver_detach+0xb8/0xc0
> [<ffffffff814547b5>] bus_remove_driver+0x55/0xd0
> [<ffffffff814560cc>] driver_unregister+0x2c/0x50
> [<ffffffff814576b2>] platform_driver_unregister+0x12/0x20
> [<ffffffffa02586c9>] cleanup_ipmi_si+0x29/0xa0 [ipmi_si]
> [<ffffffff81102100>] SyS_delete_module+0x190/0x220
> [<ffffffff8167ffee>] entry_SYSCALL_64_fastpath+0x12/0x71
> ---[ end trace 671ca97b9ac15462 ]---
>
> My platform has two BMCs (perhaps this is messing with a refcount
> somewhere), but I wonder about the ordering of this code:
>
> __device_release_driver(struct device *dev)
>
>    drv->remove(dev);
>      [ platform_drv_remove ]
>        ...
>        dev_pm_domain_detach
>          device_is_bound
>            return dev->p && klist_node_attached(&dev->p->knode_driver)
>    ...
>    klist_remove(&dev->p->knode_driver);
>
> Is the klist_remove at the bottom of __device_release_driver necessary
> to satisfy the earlier check in dev_pm_domain_detach's device_is_bound
> assertion?  If so, could these be out of order?
>
> This is core driver code, so I'm assuming it's not something as simple
> as the following (which avoided the warning on unload at least).  Any
> suggestions or extra debugging ideas welcome!  This occurs on every
> unload, so I'd be glad to test real solutions :)
>
> Thanks,
>
> -- Joe
>
> -->8--
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index c4da2df..bba54e1 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -756,6 +756,7 @@ static void __device_release_driver(struct device *dev)
>
>   		pm_runtime_put_sync(dev);
>
> +		klist_remove(&dev->p->knode_driver);
>   		if (dev->bus && dev->bus->remove)
>   			dev->bus->remove(dev);
>   		else if (drv->remove)
> @@ -767,7 +768,6 @@ static void __device_release_driver(struct device *dev)
>   			dev->pm_domain->dismiss(dev);
>   		pm_runtime_reinit(dev);
>
> -		klist_remove(&dev->p->knode_driver);
>   		device_pm_check_callbacks(dev);
>   		if (dev->bus)
>   			blocking_notifier_call_chain(&dev->bus->p->bus_notifier,