[PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Fu, Zhonghui]

Enable wiphy device to suspend/resume asynchronously. This can improve
system suspend/resume speed.

Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
 net/wireless/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 2a0bbd2..bc5e68f 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -416,6 +416,7 @@ use_default_name:
 	device_initialize(&rdev->wiphy.dev);
 	rdev->wiphy.dev.class = &ieee80211_class;
 	rdev->wiphy.dev.platform_data = rdev;
+	device_enable_async_suspend(&rdev->wiphy.dev);
 
 	INIT_LIST_HEAD(&rdev->destroy_list);
 	spin_lock_init(&rdev->destroy_list_lock);
-- 1.7.1

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Emmanuel Grumbach]

On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
<zhonghui.fu@linux.intel.com> wrote:
> Enable wiphy device to suspend/resume asynchronously. This can improve
> system suspend/resume speed.
>

How will that impact the timing with respect to the suspend call
coming from the bus?
I think that a few drivers rely on the suspend call of the wiphy
device happening before the suspend call to the bus device.
Not sure though.

> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
>  net/wireless/core.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 2a0bbd2..bc5e68f 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -416,6 +416,7 @@ use_default_name:
>         device_initialize(&rdev->wiphy.dev);
>         rdev->wiphy.dev.class = &ieee80211_class;
>         rdev->wiphy.dev.platform_data = rdev;
> +       device_enable_async_suspend(&rdev->wiphy.dev);
>
>         INIT_LIST_HEAD(&rdev->destroy_list);
>         spin_lock_init(&rdev->destroy_list_lock);
> -- 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Johannes Berg]

On Thu, 2015-07-30 at 08:55 +0300, Emmanuel Grumbach wrote:
> On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
> <zhonghui.fu@linux.intel.com> wrote:
> > Enable wiphy device to suspend/resume asynchronously. This can 
> > improve
> > system suspend/resume speed.
> > 
> 
> How will that impact the timing with respect to the suspend call
> coming from the bus?
> I think that a few drivers rely on the suspend call of the wiphy
> device happening before the suspend call to the bus device.
> 

Yes, we can't do this for precisely this reason unless we have a way to
somehow keep the dependency between the two - possibly by also marking
the other one as async (although I don't know if the async framework in
general has any FIFO guarantees, which would be required for this.)

I've dropped the patch.

johannes

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Fu, Zhonghui]

On 2015/7/30 13:55, Emmanuel Grumbach wrote:
> On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
> <zhonghui.fu@linux.intel.com> wrote:
>> Enable wiphy device to suspend/resume asynchronously. This can improve
>> system suspend/resume speed.
>>
> How will that impact the timing with respect to the suspend call
> coming from the bus?
> I think that a few drivers rely on the suspend call of the wiphy
> device happening before the suspend call to the bus device.
> Not sure though.

Sorry for late reply.

The suspend/resume timing of wiphy device and related devices will be ensured by their parent/child relationship. So, enabling wiphy device to suspend/resume asynchronously does not change any  dependency. It can only take advantage of multicore and improve system suspend/resume speed.


Thanks,
Zhonghui
>
>> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
>> ---
>>  net/wireless/core.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/wireless/core.c b/net/wireless/core.c
>> index 2a0bbd2..bc5e68f 100644
>> --- a/net/wireless/core.c
>> +++ b/net/wireless/core.c
>> @@ -416,6 +416,7 @@ use_default_name:
>>         device_initialize(&rdev->wiphy.dev);
>>         rdev->wiphy.dev.class = &ieee80211_class;
>>         rdev->wiphy.dev.platform_data = rdev;
>> +       device_enable_async_suspend(&rdev->wiphy.dev);
>>
>>         INIT_LIST_HEAD(&rdev->destroy_list);
>>         spin_lock_init(&rdev->destroy_list_lock);
>> -- 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Johannes Berg]

On Mon, 2015-08-17 at 09:48 +0800, Fu, Zhonghui wrote:
> 
> The suspend/resume timing of wiphy device and related devices will be 
> ensured by their parent/child relationship. So, enabling wiphy device 
> to suspend/resume asynchronously does not change any  dependency. It 
> can only take advantage of multicore and improve system 
> suspend/resume speed.
> 

You're going to have to explain that to me, because I don't see that.
All I see is that when looking at a device, if async is possible, it
gets added to an async work, and if async is not possible then it gets
done immediately. Even putting aside the question of whether or not
async is ordered or not (I don't know), if the wiphy is async and the
PCI (or other bus) device isn't, then it seems they could get handled
out of order, no? Or is there some magic code somewhere that I'm
missing that explicitly waits for the async of the parent/child
relationship?

johannes

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Arend van Spriel]

+ Rafael

On 08/17/2015 09:29 AM, Johannes Berg wrote:
> On Mon, 2015-08-17 at 09:48 +0800, Fu, Zhonghui wrote:
>>
>> The suspend/resume timing of wiphy device and related devices will be
>> ensured by their parent/child relationship. So, enabling wiphy device
>> to suspend/resume asynchronously does not change any  dependency. It
>> can only take advantage of multicore and improve system
>> suspend/resume speed.
>>
>
> You're going to have to explain that to me, because I don't see that.
> All I see is that when looking at a device, if async is possible, it
> gets added to an async work, and if async is not possible then it gets
> done immediately. Even putting aside the question of whether or not
> async is ordered or not (I don't know), if the wiphy is async and the
> PCI (or other bus) device isn't, then it seems they could get handled
> out of order, no? Or is there some magic code somewhere that I'm
> missing that explicitly waits for the async of the parent/child
> relationship?

This patch got me worried as well. Can't find the magic either. Maybe 
Rafael can give some hints here.

Regards,
Arend

> johannes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Fu, Zhonghui]

On 2015/8/17 16:46, Arend van Spriel wrote:
> + Rafael
>
> On 08/17/2015 09:29 AM, Johannes Berg wrote:
>> On Mon, 2015-08-17 at 09:48 +0800, Fu, Zhonghui wrote:
>>>
>>> The suspend/resume timing of wiphy device and related devices will be
>>> ensured by their parent/child relationship. So, enabling wiphy device
>>> to suspend/resume asynchronously does not change any  dependency. It
>>> can only take advantage of multicore and improve system
>>> suspend/resume speed.
>>>
>>
>> You're going to have to explain that to me, because I don't see that.
>> All I see is that when looking at a device, if async is possible, it
>> gets added to an async work, and if async is not possible then it gets
>> done immediately. Even putting aside the question of whether or not
>> async is ordered or not (I don't know), if the wiphy is async and the
>> PCI (or other bus) device isn't, then it seems they could get handled
>> out of order, no? Or is there some magic code somewhere that I'm
>> missing that explicitly waits for the async of the parent/child
>> relationship?
>
> This patch got me worried as well. Can't find the magic either. Maybe Rafael can give some hints here.

"dpm_wait_for_children" function will be invoked in "__device_suspend", "__device_suspend_late", and "__device_suspend_noirq" functions to synchronize the child relationship. "dpm_wait" function will be invoked in "device_resume_noirq", "device_resume_early", and "device_resume" functions to synchronize the parent relationship. If two devices have parent/child relationship, but different suspend/resume mode(sync or async), this will have no impact to PM timing order between them. Because all devices will use "__device_suspend", "__device_suspend_late" ... functions to complete their PM transition.


Thanks,
Zhonghui 
>
> Regards,
> Arend
>
>> johannes
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Johannes Berg]

On Mon, 2015-08-24 at 11:45 +0800, Fu, Zhonghui wrote:
> 
> On 2015/8/17 16:46, Arend van Spriel wrote:
> > + Rafael
> > 
> > On 08/17/2015 09:29 AM, Johannes Berg wrote:
> > > On Mon, 2015-08-17 at 09:48 +0800, Fu, Zhonghui wrote:
> > > > 
> > > > The suspend/resume timing of wiphy device and related devices 
> > > > will be
> > > > ensured by their parent/child relationship. So, enabling wiphy 
> > > > device
> > > > to suspend/resume asynchronously does not change any 
> > > >  dependency. It
> > > > can only take advantage of multicore and improve system
> > > > suspend/resume speed.
> > > > 
> > > 
> > > You're going to have to explain that to me, because I don't see 
> > > that.
> > > All I see is that when looking at a device, if async is possible, 
> > > it
> > > gets added to an async work, and if async is not possible then it 
> > > gets
> > > done immediately. Even putting aside the question of whether or 
> > > not
> > > async is ordered or not (I don't know), if the wiphy is async and 
> > > the
> > > PCI (or other bus) device isn't, then it seems they could get 
> > > handled
> > > out of order, no? Or is there some magic code somewhere that I'm
> > > missing that explicitly waits for the async of the parent/child
> > > relationship?
> > 
> > This patch got me worried as well. Can't find the magic either. 
> > Maybe Rafael can give some hints here.
> 
> "dpm_wait_for_children" function will be invoked in 
> "__device_suspend", "__device_suspend_late", and 
> "__device_suspend_noirq" functions to synchronize the child 
> relationship. "dpm_wait" function will be invoked in 
> "device_resume_noirq", "device_resume_early", and "device_resume" 
> functions to synchronize the parent relationship. If two devices have 
> parent/child relationship, but different suspend/resume mode(sync or 
> async), this will have no impact to PM timing order between them. 
> Because all devices will use "__device_suspend", 
> "__device_suspend_late" ... functions to complete their PM 
> transition.
> 

Ok, good point. For the unaware here, can you please resend with a
commit message amended with some of this information?

thanks,
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net/wireless: enable wiphy device to suspend/resume asynchronously

[Fu, Zhonghui]

On 2015/8/24 15:38, Johannes Berg wrote:
> On Mon, 2015-08-24 at 11:45 +0800, Fu, Zhonghui wrote:
>> On 2015/8/17 16:46, Arend van Spriel wrote:
>>> + Rafael
>>>
>>> On 08/17/2015 09:29 AM, Johannes Berg wrote:
>>>> On Mon, 2015-08-17 at 09:48 +0800, Fu, Zhonghui wrote:
>>>>> The suspend/resume timing of wiphy device and related devices 
>>>>> will be
>>>>> ensured by their parent/child relationship. So, enabling wiphy 
>>>>> device
>>>>> to suspend/resume asynchronously does not change any 
>>>>>  dependency. It
>>>>> can only take advantage of multicore and improve system
>>>>> suspend/resume speed.
>>>>>
>>>> You're going to have to explain that to me, because I don't see 
>>>> that.
>>>> All I see is that when looking at a device, if async is possible, 
>>>> it
>>>> gets added to an async work, and if async is not possible then it 
>>>> gets
>>>> done immediately. Even putting aside the question of whether or 
>>>> not
>>>> async is ordered or not (I don't know), if the wiphy is async and 
>>>> the
>>>> PCI (or other bus) device isn't, then it seems they could get 
>>>> handled
>>>> out of order, no? Or is there some magic code somewhere that I'm
>>>> missing that explicitly waits for the async of the parent/child
>>>> relationship?
>>> This patch got me worried as well. Can't find the magic either. 
>>> Maybe Rafael can give some hints here.
>> "dpm_wait_for_children" function will be invoked in 
>> "__device_suspend", "__device_suspend_late", and 
>> "__device_suspend_noirq" functions to synchronize the child 
>> relationship. "dpm_wait" function will be invoked in 
>> "device_resume_noirq", "device_resume_early", and "device_resume" 
>> functions to synchronize the parent relationship. If two devices have 
>> parent/child relationship, but different suspend/resume mode(sync or 
>> async), this will have no impact to PM timing order between them. 
>> Because all devices will use "__device_suspend", 
>> "__device_suspend_late" ... functions to complete their PM 
>> transition.
>>
> Ok, good point. For the unaware here, can you please resend with a
> commit message amended with some of this information?
I take some leaves these days, so very sorry for late reply.

I have resent this patch with your advices - "[PATCH v2] net/wireless: enable wiphy device to suspend/resume asynchronously".


Thanks,
Zhonghui
>
> thanks,
> johannes