| Message ID | 55B9B3BA.6080406@linux.intel.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
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 -- 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
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 -- 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
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 -- 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
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 -- 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
+ 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 > -- 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
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 -- 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
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- 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
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);
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(-) -- 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