2015-07-30 05:18:55

by Fu, Zhonghui

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

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

Signed-off-by: Zhonghui Fu <[email protected]>
---
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



2015-07-30 05:55:52

by Emmanuel Grumbach

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

On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
<[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-08-24 03:46:00

by Fu, Zhonghui

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



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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2015-08-13 13:13:49

by Johannes Berg

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

On Thu, 2015-07-30 at 08:55 +0300, Emmanuel Grumbach wrote:
> On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
> <[email protected]> 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

2015-08-17 08:46:33

by Arend van Spriel

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

+ 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2015-08-24 07:38:47

by Johannes Berg

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

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

2015-08-17 01:48:28

by Fu, Zhonghui

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



On 2015/7/30 13:55, Emmanuel Grumbach wrote:
> On Thu, Jul 30, 2015 at 8:18 AM, Fu, Zhonghui
> <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2015-08-17 07:29:22

by Johannes Berg

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

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

2015-09-19 02:45:15

by Fu, Zhonghui

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



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