Re: [lkp] [PM / sleep] 013c074f86: [drm:intel_opregion_init [i915]] *ERROR* No ACPI video bus found

Re: [lkp] [PM / sleep] 013c074f86: [drm:intel_opregion_init [i915]] *ERROR* No ACPI video bus found

[Grygorii Strashko]

Hi All,

Cc: more people + linux-input@vger.kernel.org

On 01/07/2016 03:49 AM, kernel test robot wrote:
> FYI, we noticed the below changes on
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> commit 013c074f8642d8e815ad670601f8e27155a74b57 ("PM / sleep: prohibit devices probing during suspend/hibernation")
> 
> <4>[   33.021805] serio serio0: device_attach() failed for isa0060/serio0 (i8042 KBD port), error: -517
> <4>[   33.021813] serio serio1: device_attach() failed for isa0060/serio1 (i8042 AUX port), error: -517
> <6>[   33.021906] PM: resume of devices complete after 171.688 msecs
> 
> To reproduce:
> 
>          git clone git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git
>          cd lkp-tests
>          bin/lkp install job.yaml  # job file is attached in this email
>          bin/lkp run     job.yaml

I've done some investigation of the above log output:
1) Small introduction first: commit 013c074f8642 was created, because it was identified
that it's unsafe to bind/probe drivers during suspend resume, mainly because this will
cause changing of pointers on PM data structures on the fly for devices which already
tracked by Suspend core.

2) above log prints can be generated by the below call chain (I've checked the code only):

drivers/input/serio/serio.c
serio_resume()
 - serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT)
   - queue_work(system_long_wq, &serio_event_work);
 ....
 - serio_handle_event()
   - serio_reconnect_port()
	- serio_reconnect_driver() --- return <0
		
	- serio_disconnect_port()
	  - device_release_driver() --- unbind
 	- serio_find_driver()
	   error = device_attach(&serio->dev);
	   if (error < 0)
		dev_warn(&serio->dev,
			 "device_attach() failed for %s (%s), error: %d\n",
			 serio->phys, serio->name, error);

  device_attach() will return -EPROBE_DEFER(-517) because the device probing is not allowed yet.

As per my understanding (and it's not clear from issue description), above situation
should not break functionality of the system - just device's probe will be delayed till the
end of dpm_complete(). So, I propose consider the option - do not print above warnings if 
return code from device_attach() is -EPROBE_DEFER.

diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index 8f82897..1ca7f55 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -134,7 +134,7 @@ static void serio_find_driver(struct serio *serio)
        int error;
 
        error = device_attach(&serio->dev);
-       if (error < 0)
+       if (error < 0 && error != -EPROBE_DEFER)
                dev_warn(&serio->dev,
                         "device_attach() failed for %s (%s), error: %d\n",
                         serio->phys, serio->name, error);


[1] https://lkml.org/lkml/2015/9/11/554
-- 
regards,
-grygorii

Re: [lkp] [PM / sleep] 013c074f86: [drm:intel_opregion_init [i915]] *ERROR* No ACPI video bus found

[Rafael J. Wysocki]

On Tuesday, January 12, 2016 08:38:57 PM Grygorii Strashko wrote:
> Hi All,
> 
> Cc: more people + linux-input@vger.kernel.org
> 
> On 01/07/2016 03:49 AM, kernel test robot wrote:
> > FYI, we noticed the below changes on
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > commit 013c074f8642d8e815ad670601f8e27155a74b57 ("PM / sleep: prohibit devices probing during suspend/hibernation")
> > 
> > <4>[   33.021805] serio serio0: device_attach() failed for isa0060/serio0 (i8042 KBD port), error: -517
> > <4>[   33.021813] serio serio1: device_attach() failed for isa0060/serio1 (i8042 AUX port), error: -517
> > <6>[   33.021906] PM: resume of devices complete after 171.688 msecs
> > 
> > To reproduce:
> > 
> >          git clone git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git
> >          cd lkp-tests
> >          bin/lkp install job.yaml  # job file is attached in this email
> >          bin/lkp run     job.yaml
> 
> I've done some investigation of the above log output:
> 1) Small introduction first: commit 013c074f8642 was created, because it was identified
> that it's unsafe to bind/probe drivers during suspend resume, mainly because this will
> cause changing of pointers on PM data structures on the fly for devices which already
> tracked by Suspend core.
> 
> 2) above log prints can be generated by the below call chain (I've checked the code only):
> 
> drivers/input/serio/serio.c
> serio_resume()
>  - serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT)
>    - queue_work(system_long_wq, &serio_event_work);
>  ....
>  - serio_handle_event()
>    - serio_reconnect_port()
> 	- serio_reconnect_driver() --- return <0
> 		
> 	- serio_disconnect_port()
> 	  - device_release_driver() --- unbind
>  	- serio_find_driver()
> 	   error = device_attach(&serio->dev);
> 	   if (error < 0)
> 		dev_warn(&serio->dev,
> 			 "device_attach() failed for %s (%s), error: %d\n",
> 			 serio->phys, serio->name, error);
> 
>   device_attach() will return -EPROBE_DEFER(-517) because the device probing is not allowed yet.
> 
> As per my understanding (and it's not clear from issue description), above situation
> should not break functionality of the system - just device's probe will be delayed till the
> end of dpm_complete(). So, I propose consider the option - do not print above warnings if 
> return code from device_attach() is -EPROBE_DEFER.

This looks like a reasonable thing to do to me.

> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index 8f82897..1ca7f55 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -134,7 +134,7 @@ static void serio_find_driver(struct serio *serio)
>         int error;
>  
>         error = device_attach(&serio->dev);
> -       if (error < 0)
> +       if (error < 0 && error != -EPROBE_DEFER)
>                 dev_warn(&serio->dev,
>                          "device_attach() failed for %s (%s), error: %d\n",
>                          serio->phys, serio->name, error);
> 
> 
> [1] https://lkml.org/lkml/2015/9/11/554

Thanks,
Rafael

Re: [lkp] [PM / sleep] 013c074f86: [drm:intel_opregion_init [i915]] *ERROR* No ACPI video bus found

[Dmitry Torokhov]

On Tue, Jan 12, 2016 at 09:11:15PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, January 12, 2016 08:38:57 PM Grygorii Strashko wrote:
> > Hi All,
> > 
> > Cc: more people + linux-input@vger.kernel.org
> > 
> > On 01/07/2016 03:49 AM, kernel test robot wrote:
> > > FYI, we noticed the below changes on
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > commit 013c074f8642d8e815ad670601f8e27155a74b57 ("PM / sleep: prohibit devices probing during suspend/hibernation")
> > > 
> > > <4>[   33.021805] serio serio0: device_attach() failed for isa0060/serio0 (i8042 KBD port), error: -517
> > > <4>[   33.021813] serio serio1: device_attach() failed for isa0060/serio1 (i8042 AUX port), error: -517
> > > <6>[   33.021906] PM: resume of devices complete after 171.688 msecs
> > > 
> > > To reproduce:
> > > 
> > >          git clone git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git
> > >          cd lkp-tests
> > >          bin/lkp install job.yaml  # job file is attached in this email
> > >          bin/lkp run     job.yaml
> > 
> > I've done some investigation of the above log output:
> > 1) Small introduction first: commit 013c074f8642 was created, because it was identified
> > that it's unsafe to bind/probe drivers during suspend resume, mainly because this will
> > cause changing of pointers on PM data structures on the fly for devices which already
> > tracked by Suspend core.
> > 
> > 2) above log prints can be generated by the below call chain (I've checked the code only):
> > 
> > drivers/input/serio/serio.c
> > serio_resume()
> >  - serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT)
> >    - queue_work(system_long_wq, &serio_event_work);
> >  ....
> >  - serio_handle_event()
> >    - serio_reconnect_port()
> > 	- serio_reconnect_driver() --- return <0

Any chance you could track why serio_reconnect_driver returns error
here? Ideally it should have succeeded.

> > 		
> > 	- serio_disconnect_port()
> > 	  - device_release_driver() --- unbind
> >  	- serio_find_driver()
> > 	   error = device_attach(&serio->dev);
> > 	   if (error < 0)
> > 		dev_warn(&serio->dev,
> > 			 "device_attach() failed for %s (%s), error: %d\n",
> > 			 serio->phys, serio->name, error);
> > 
> >   device_attach() will return -EPROBE_DEFER(-517) because the device probing is not allowed yet.
> > 
> > As per my understanding (and it's not clear from issue description), above situation
> > should not break functionality of the system - just device's probe will be delayed till the
> > end of dpm_complete(). So, I propose consider the option - do not print above warnings if 
> > return code from device_attach() is -EPROBE_DEFER.
> 
> This looks like a reasonable thing to do to me.

Looks like it will work.

Thanks.

-- 
Dmitry

Re: [lkp] [PM / sleep] 013c074f86: [drm:intel_opregion_init [i915]] *ERROR* No ACPI video bus found

[Grygorii Strashko]

On 01/12/2016 10:25 PM, Dmitry Torokhov wrote:
> On Tue, Jan 12, 2016 at 09:11:15PM +0100, Rafael J. Wysocki wrote:
>> On Tuesday, January 12, 2016 08:38:57 PM Grygorii Strashko wrote:
>>> Cc: more people + linux-input@vger.kernel.org
>>>
>>> On 01/07/2016 03:49 AM, kernel test robot wrote:
>>>> FYI, we noticed the below changes on
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>>>> commit 013c074f8642d8e815ad670601f8e27155a74b57 ("PM / sleep: prohibit devices probing during suspend/hibernation")
>>>>
>>>> <4>[   33.021805] serio serio0: device_attach() failed for isa0060/serio0 (i8042 KBD port), error: -517
>>>> <4>[   33.021813] serio serio1: device_attach() failed for isa0060/serio1 (i8042 AUX port), error: -517
>>>> <6>[   33.021906] PM: resume of devices complete after 171.688 msecs
>>>>
>>>> To reproduce:
>>>>
>>>>           git clone git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git
>>>>           cd lkp-tests
>>>>           bin/lkp install job.yaml  # job file is attached in this email
>>>>           bin/lkp run     job.yaml
>>>
>>> I've done some investigation of the above log output:
>>> 1) Small introduction first: commit 013c074f8642 was created, because it was identified
>>> that it's unsafe to bind/probe drivers during suspend resume, mainly because this will
>>> cause changing of pointers on PM data structures on the fly for devices which already
>>> tracked by Suspend core.
>>>
>>> 2) above log prints can be generated by the below call chain (I've checked the code only):
>>>
>>> drivers/input/serio/serio.c
>>> serio_resume()
>>>   - serio_queue_event(serio, NULL, SERIO_RECONNECT_PORT)
>>>     - queue_work(system_long_wq, &serio_event_work);
>>>   ....
>>>   - serio_handle_event()
>>>     - serio_reconnect_port()
>>> 	- serio_reconnect_driver() --- return <0
> 
> Any chance you could track why serio_reconnect_driver returns error
> here? Ideally it should have succeeded.

Sry, I'm not absolutely sure that exactly this path generates warnings :(
Issue is reported for x86 and i8042 which are not my current dev Platforms,
so this analyze is based on code review only.

As per code, there are few more places which might call serio_reconnect_driver/serio_find_driver
- SERIO_RESCAN_PORT/serio_rescan() 
- SERIO_RECONNECT_SUBTREE/serio_reconnect() - called by amba_kmi_resume()


> 
>>> 		
>>> 	- serio_disconnect_port()
>>> 	  - device_release_driver() --- unbind
>>>   	- serio_find_driver()
>>> 	   error = device_attach(&serio->dev);
>>> 	   if (error < 0)
>>> 		dev_warn(&serio->dev,
>>> 			 "device_attach() failed for %s (%s), error: %d\n",
>>> 			 serio->phys, serio->name, error);
>>>
>>>    device_attach() will return -EPROBE_DEFER(-517) because the device probing is not allowed yet.
>>>
>>> As per my understanding (and it's not clear from issue description), above situation
>>> should not break functionality of the system - just device's probe will be delayed till the
>>> end of dpm_complete(). So, I propose consider the option - do not print above warnings if
>>> return code from device_attach() is -EPROBE_DEFER.
>>
>> This looks like a reasonable thing to do to me.
> 
> Looks like it will work.

I'll send the patch then.


-- 
regards,
-grygorii