Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
> Sure, add these test results to the patch as well showing reduced wakeups. > > I would say submit the freezable_schedule as a single separate patch > independent of the vsoc series since it can go in separately, and also > benefits other things than vsoc. > > Also CC Rafael (power maintainer) on it. Thanks, I have splitted the patch set[0][1] and submitted the freezable_schedule patch separately (only cc-ing people responsible for the wait api + Rafael). regards, Hugo [0] https://lkml.org/lkml/2019/2/7/802 [1] https://lkml.org/lkml/2019/2/7/870 -- Hugo Lefeuvre (hle)|www.owl.eu.com RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C signature.asc Description: PGP signature
Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
On Thu, Feb 07, 2019 at 12:30:50AM +0100, Hugo Lefeuvre wrote: > Hi Joel, > > > I'm curious did you try the freezing process and see if pointless wakeups > > are > > reduced? That would be an added bonus if you did. > > I'm currently testing these changes. I hope to be able to come back with > more concrete results soon. > > Also, I just noticed that the third patch removes a necessary #include > . I will submit an updated version tomorrow. > > Thanks for the review! Sure, add these test results to the patch as well showing reduced wakeups. I would say submit the freezable_schedule as a single separate patch independent of the vsoc series since it can go in separately, and also benefits other things than vsoc. Also CC Rafael (power maintainer) on it. Thank you! - Joel
Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
Hi,
> > The result is a potential performance gain during freeze, since less
> > tasks have to be awaken.
>
> I'm curious did you try the freezing process and see if pointless wakeups are
> reduced? That would be an added bonus if you did.
Test env: fresh Debian QEMU vm with 4.19 stable kernel.
Test process:
- Added two debug logs to freeze_task:
bool freeze_task(struct task_struct *p)
{
unsigned long flags;
[snip]
pr_info("freezing a task");
[snip]
if (freezer_should_skip(p)) {
pr_info("skeeping a task");
return false;
}
[snip]
}
- Triggered manual freeze:
# echo freezer > /sys/power/pm_test
# echo test_resume > /sys/power/disk
# echo disk > /sys/power/state
- grep -c to get the number of "freezing a task" and "skeeping a task"
lines in kern.log.
Results:
Without my patch: 448 calls freeze_task, 12 skipped.
With my patch: 448 calls, 32 skipped.
2.6x more tasks skipped.
Not sure this is the best way to test this patch, though. Any advice?
regards,
Hugo
--
Hugo Lefeuvre (hle)|www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C
signature.asc
Description: PGP signature
Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
Hi Joel, > I'm curious did you try the freezing process and see if pointless wakeups are > reduced? That would be an added bonus if you did. I'm currently testing these changes. I hope to be able to come back with more concrete results soon. Also, I just noticed that the third patch removes a necessary #include . I will submit an updated version tomorrow. Thanks for the review! regards, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C
Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
On Fri, Feb 01, 2019 at 06:38:05AM +0100, Hugo Lefeuvre wrote:
> Replace schedule(); try_to_freeze() by freezable_schedule().
>
> Tasks calling freezable_schedule() set the PF_FREEZER_SKIP flag
> before calling schedule(). Unlike tasks calling schedule();
> try_to_freeze() tasks calling freezable_schedule() are not awaken by
> try_to_freeze_tasks(). Instead they call try_to_freeze() when they
> wake up if the freeze is still underway.
>
> It is not a problem since sleeping tasks can't do anything which isn't
> allowed for a frozen task while sleeping.
>
> The result is a potential performance gain during freeze, since less
> tasks have to be awaken.
Reviewed-by: Joel Fernandes (Google)
thanks,
- Joel
>
> Signed-off-by: Hugo Lefeuvre
> ---
> include/linux/wait.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index ed7c122cb31f..5f3efabc36f4 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -308,7 +308,7 @@ do {
> \
>
> #define __wait_event_freezable(wq_head, condition)
> \
> ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,
> \
> - schedule(); try_to_freeze())
> + freezable_schedule())
>
> /**
> * wait_event_freezable - sleep (or freeze) until a condition gets true
> @@ -367,7 +367,7 @@ do {
> \
> #define __wait_event_freezable_timeout(wq_head, condition, timeout)
> \
> ___wait_event(wq_head, ___wait_cond_timeout(condition),
> \
> TASK_INTERRUPTIBLE, 0, timeout,
> \
> - __ret = schedule_timeout(__ret); try_to_freeze())
> + __ret = freezable_schedule_timeout(__ret))
>
> /*
> * like wait_event_timeout() -- except it uses TASK_INTERRUPTIBLE to avoid
> @@ -588,7 +588,7 @@ do {
> \
>
> #define __wait_event_freezable_exclusive(wq, condition)
> \
> ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 1, 0,
> \
> - schedule(); try_to_freeze())
> + freezable_schedule())
>
> #define wait_event_freezable_exclusive(wq, condition)
> \
> ({
> \
> --
> 2.20.1
Re: [PATCH 1/3] sched/wait: use freezable_schedule when possible
On Fri, Feb 01, 2019 at 06:38:05AM +0100, Hugo Lefeuvre wrote: > Replace schedule(); try_to_freeze() by freezable_schedule(). > > Tasks calling freezable_schedule() set the PF_FREEZER_SKIP flag > before calling schedule(). Unlike tasks calling schedule(); > try_to_freeze() tasks calling freezable_schedule() are not awaken by > try_to_freeze_tasks(). Instead they call try_to_freeze() when they > wake up if the freeze is still underway. > > It is not a problem since sleeping tasks can't do anything which isn't > allowed for a frozen task while sleeping. > > The result is a potential performance gain during freeze, since less > tasks have to be awaken. I'm curious did you try the freezing process and see if pointless wakeups are reduced? That would be an added bonus if you did. Otherwise seems like a nice change. Peter and Rafael, what do you think? thanks, - Joel
[PATCH 1/3] sched/wait: use freezable_schedule when possible
Replace schedule(); try_to_freeze() by freezable_schedule().
Tasks calling freezable_schedule() set the PF_FREEZER_SKIP flag
before calling schedule(). Unlike tasks calling schedule();
try_to_freeze() tasks calling freezable_schedule() are not awaken by
try_to_freeze_tasks(). Instead they call try_to_freeze() when they
wake up if the freeze is still underway.
It is not a problem since sleeping tasks can't do anything which isn't
allowed for a frozen task while sleeping.
The result is a potential performance gain during freeze, since less
tasks have to be awaken.
Signed-off-by: Hugo Lefeuvre
---
include/linux/wait.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/wait.h b/include/linux/wait.h
index ed7c122cb31f..5f3efabc36f4 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -308,7 +308,7 @@ do {
\
#define __wait_event_freezable(wq_head, condition)
\
___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,
\
- schedule(); try_to_freeze())
+ freezable_schedule())
/**
* wait_event_freezable - sleep (or freeze) until a condition gets true
@@ -367,7 +367,7 @@ do {
\
#define __wait_event_freezable_timeout(wq_head, condition, timeout)
\
___wait_event(wq_head, ___wait_cond_timeout(condition),
\
TASK_INTERRUPTIBLE, 0, timeout,
\
- __ret = schedule_timeout(__ret); try_to_freeze())
+ __ret = freezable_schedule_timeout(__ret))
/*
* like wait_event_timeout() -- except it uses TASK_INTERRUPTIBLE to avoid
@@ -588,7 +588,7 @@ do {
\
#define __wait_event_freezable_exclusive(wq, condition)
\
___wait_event(wq, condition, TASK_INTERRUPTIBLE, 1, 0,
\
- schedule(); try_to_freeze())
+ freezable_schedule())
#define wait_event_freezable_exclusive(wq, condition)
\
({
\
--
2.20.1
