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

[Corey Minyard <minyard@acm.org>]

Tomeu, you added that check in

[989561de9b5112999475b406557d9c7e9e59c041] PM / Domains: add setter for 
dev.pm_domain

and either something is wrong in the platform device handling or 
elsewhere in the device code, if
that check is valid.

-corey

On 01/28/2016 07:56 AM, Corey Minyard wrote:
> 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,
>