net/wireless: enable wiphy device to suspend/resume asynchronously

PATCH In review Fu, Zhonghui 4 participants 9 messages Jul 30, 2015 – Sep 19, 2015

Subsystems (3)

Networking Network drivers 802.11 (including cfg80211/NL80211)

lore

Share & tools

lore

Emmanuel Grumbach

2015-07-30 05:55 UTC #1 lore raw

Quoted 3 lines · Zhonghui

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.

Quoted 23 lines

Signed-off-by Zhonghui Fu <[email protected]>

---
 net/wireless/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

net/wireless/core.c +1 -2

index 2a0bbd2..bc5e68f 100644
@@ -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

Johannes Berg

maintainer

2015-08-13 13:13 UTC #2 lore raw

Quoted 7 lines · Emmanuel Grumbach

Quoted 4 lines · Zhonghui
Enable wiphy device to suspend/resume asynchronously. This can improve
system suspend/resume speed.

Quoted 5 lines

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

Fu, Zhonghui

author

2015-08-17 01:48 UTC #3 lore raw

In reply to Emmanuel Grumbach

Quoted 10 lines · Emmanuel Grumbach

Quoted 3 lines · Zhonghui
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

Quoted 28 lines

Quoted 21 lines
Signed-off-by Zhonghui Fu <[email protected]>
---
 net/wireless/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
net/wireless/core.c +1 -2
index 2a0bbd2..bc5e68f 100644
@@ -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
 
--
Quoted 3 lines
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

Johannes Berg

maintainer

2015-08-17 07:29 UTC #4 lore raw

Quoted 7 lines · Zhonghui

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

Arend van Spriel

2015-08-17 08:46 UTC #5 lore raw

+ Rafael

Quoted 9 lines · Johannes Berg

Quoted 7 lines · Zhonghui
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.

Quoted 9 lines

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

Quoted 6 lines

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

Fu, Zhonghui

author

2015-08-24 03:45 UTC #6 lore raw

Quoted 22 lines · Arend van Spriel

+ Rafael
Quoted 9 lines · Johannes Berg
Quoted 7 lines · Zhonghui
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.
Quoted 9 lines
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?

Quoted 1 line

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

Quoted 15 lines

Regards,
Arend

Quoted 6 lines

johannes

Signature · 3 lines

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

Signature · 3 lines

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

Johannes Berg

maintainer

2015-08-24 07:38 UTC #7 lore raw

Quoted 36 lines · Zhonghui

Quoted 31 lines · Arend van Spriel
+ Rafael
Quoted 12 lines · Johannes Berg
Quoted 10 lines · Zhonghui
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.
Quoted 15 lines
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?
Quoted 2 lines
This patch got me worried as well. Can't find the magic either. Maybe Rafael can give some hints here.

Quoted 12 lines

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

Fu, Zhonghui

author

2015-09-19 02:45 UTC #8 lore raw

Quoted 44 lines · Johannes Berg

Quoted 31 lines · Zhonghui
Quoted 28 lines · Arend van Spriel
+ Rafael
Quoted 10 lines · Johannes Berg
Quoted 9 lines · Zhonghui
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.
Quoted 15 lines
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?
Quoted 2 lines
This patch got me worried as well. Can't find the magic either. Maybe Rafael can give some hints here.
Quoted 12 lines
"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.

Quoted 2 lines

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

Quoted 3 lines

thanks,
johannes