* [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set [not found] ` <1367271946-7239-1-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> @ 2013-04-29 21:45 ` Colin Cross [not found] ` <1367271946-7239-3-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Colin Cross @ 2013-04-29 21:45 UTC (permalink / raw) To: linux-pm-u79uwXL29TY76Z2rM5mHXA Cc: Len Brown, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rafael J. Wysocki, arve-z5hGa2qSFaRBDgjK7y7TUQ, Pavel Machek, Colin Cross, Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA If a task has called freezer_do_not_count(), don't bother waking it up. If it happens to wake up later it will call freezer_count() and immediately enter the refrigerator. Signed-off-by: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> --- kernel/cgroup_freezer.c | 5 ++++- kernel/power/process.c | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c index 75dda1e..406dd71 100644 --- a/kernel/cgroup_freezer.c +++ b/kernel/cgroup_freezer.c @@ -331,8 +331,11 @@ static void freeze_cgroup(struct freezer *freezer) struct task_struct *task; cgroup_iter_start(cgroup, &it); - while ((task = cgroup_iter_next(cgroup, &it))) + while ((task = cgroup_iter_next(cgroup, &it))) { + if (freezer_should_skip(task)) + continue; freeze_task(task); + } cgroup_iter_end(cgroup, &it); } diff --git a/kernel/power/process.c b/kernel/power/process.c index eb7e6c1..0680be2 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -46,10 +46,10 @@ static int try_to_freeze_tasks(bool user_only) todo = 0; read_lock(&tasklist_lock); do_each_thread(g, p) { - if (p == current || !freeze_task(p)) + if (p == current || freezer_should_skip(p)) continue; - if (!freezer_should_skip(p)) + if (freeze_task(p)) todo++; } while_each_thread(g, p); read_unlock(&tasklist_lock); -- 1.8.2.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <1367271946-7239-3-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set [not found] ` <1367271946-7239-3-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> @ 2013-04-29 21:51 ` Tejun Heo [not found] ` <20130429215157.GA2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-04-29 21:51 UTC (permalink / raw) To: Colin Cross Cc: Len Brown, linux-pm-u79uwXL29TY76Z2rM5mHXA, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rafael J. Wysocki, arve-z5hGa2qSFaRBDgjK7y7TUQ, Pavel Machek, cgroups-u79uwXL29TY76Z2rM5mHXA Hello, On Mon, Apr 29, 2013 at 02:45:38PM -0700, Colin Cross wrote: > If a task has called freezer_do_not_count(), don't bother waking it > up. If it happens to wake up later it will call freezer_count() and > immediately enter the refrigerator. > > Signed-off-by: Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org> > --- > kernel/cgroup_freezer.c | 5 ++++- > kernel/power/process.c | 4 ++-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c > index 75dda1e..406dd71 100644 > --- a/kernel/cgroup_freezer.c > +++ b/kernel/cgroup_freezer.c > @@ -331,8 +331,11 @@ static void freeze_cgroup(struct freezer *freezer) > struct task_struct *task; > > cgroup_iter_start(cgroup, &it); > - while ((task = cgroup_iter_next(cgroup, &it))) > + while ((task = cgroup_iter_next(cgroup, &it))) { > + if (freezer_should_skip(task)) > + continue; > freeze_task(task); > + } > cgroup_iter_end(cgroup, &it); I feel a bit weary of changes which try to optimize state checks for freezer because the synchronization rules are kinda fragile and things may not work reliably depending on who's testing the flag, and it has been subtly broken in various ways in the past (maybe even now). Can you please explain the benefits of this patch (in terms of actual overhead because not many use freezer_do_not_count()) and why this is correct? Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20130429215157.GA2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>]
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set [not found] ` <20130429215157.GA2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org> @ 2013-04-29 21:57 ` Tejun Heo 2013-04-29 22:02 ` Colin Cross 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-04-29 21:57 UTC (permalink / raw) To: Colin Cross Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rafael J. Wysocki, arve-z5hGa2qSFaRBDgjK7y7TUQ, Li Zefan, Len Brown, Pavel Machek, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, cgroups-u79uwXL29TY76Z2rM5mHXA On Mon, Apr 29, 2013 at 02:51:57PM -0700, Tejun Heo wrote: > I feel a bit weary of changes which try to optimize state checks for > freezer because the synchronization rules are kinda fragile and things > may not work reliably depending on who's testing the flag, and it has > been subtly broken in various ways in the past (maybe even now). Can And BTW, this is why the function is only used when checking whether a task is frozen rather than to decide to issue freeze_task() on it, and it seems your change is correct now that we don't have per-task FREEZE flag but I can't say I like the change. I'd really like to keep things as dumb as possible for freezer. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set 2013-04-29 21:57 ` Tejun Heo @ 2013-04-29 22:02 ` Colin Cross [not found] ` <CAMbhsRQHkm=XTYtummEZq1h4sa-tRGwLpk8Uyhpj9D3+jQF3dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Colin Cross @ 2013-04-29 22:02 UTC (permalink / raw) To: Tejun Heo Cc: Linux PM list, lkml, Rafael J. Wysocki, Arve Hjønnevåg, Li Zefan, Len Brown, Pavel Machek, containers, cgroups On Mon, Apr 29, 2013 at 2:57 PM, Tejun Heo <tj@kernel.org> wrote: > On Mon, Apr 29, 2013 at 02:51:57PM -0700, Tejun Heo wrote: >> I feel a bit weary of changes which try to optimize state checks for >> freezer because the synchronization rules are kinda fragile and things >> may not work reliably depending on who's testing the flag, and it has >> been subtly broken in various ways in the past (maybe even now). Can > > And BTW, this is why the function is only used when checking whether a > task is frozen rather than to decide to issue freeze_task() on it, and > it seems your change is correct now that we don't have per-task FREEZE > flag but I can't say I like the change. I'd really like to keep > things as dumb as possible for freezer. See the first patch in the series (which isn't available in the archive yet, so I can't link to it). The short version is that Android goes through suspend/resume very often (every few seconds when on a busy wifi network with the screen off), and a significant portion of the energy used to go in and out of suspend is spent in the freezer. This patch series takes the most common userspace sleep points and converts them to PF_FREEZER_SKIP, which reduces the number of context switches for every suspend or resume event on a freshly-booted Android device from 1000 to 25, and reduces the time spent freezing by a factor of 5. It will have a similar effect on a non-Android system, although those generally don't care about suspend/resume optimization. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <CAMbhsRQHkm=XTYtummEZq1h4sa-tRGwLpk8Uyhpj9D3+jQF3dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set [not found] ` <CAMbhsRQHkm=XTYtummEZq1h4sa-tRGwLpk8Uyhpj9D3+jQF3dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2013-04-29 22:08 ` Tejun Heo [not found] ` <20130429220831.GC2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-04-29 22:08 UTC (permalink / raw) To: Colin Cross Cc: Len Brown, Linux PM list, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, lkml, Rafael J. Wysocki, Arve Hjønnevåg, Pavel Machek, cgroups-u79uwXL29TY76Z2rM5mHXA Hey, On Mon, Apr 29, 2013 at 03:02:19PM -0700, Colin Cross wrote: > See the first patch in the series (which isn't available in the > archive yet, so I can't link to it). The short version is that It didn't arrive in my lkml folder either. Maybe vger is taking some time distributing emails. > Android goes through suspend/resume very often (every few seconds when > on a busy wifi network with the screen off), and a significant portion > of the energy used to go in and out of suspend is spent in the > freezer. This patch series takes the most common userspace sleep > points and converts them to PF_FREEZER_SKIP, which reduces the number > of context switches for every suspend or resume event on a > freshly-booted Android device from 1000 to 25, and reduces the time Ah, okay, so you're spreading PF_FREEZER_SKIP. When you post patches which touch the freezer can you please cc me and Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> (I'll ping him this time)? Freezer has been very subtly broken in various ways and many kthread users are still broken, so let's tread carefully. > spent freezing by a factor of 5. It will have a similar effect on a > non-Android system, although those generally don't care about > suspend/resume optimization. Yeah, if it's something which makes actual difference rather than "this seems to be a good idea" thing, sure, let's find a way. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20130429220831.GC2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>]
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set [not found] ` <20130429220831.GC2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org> @ 2013-04-29 22:16 ` Tejun Heo 2013-04-30 11:48 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2013-04-29 22:16 UTC (permalink / raw) To: Colin Cross Cc: Len Brown, Linux PM list, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, lkml, Rafael J. Wysocki, Arve Hjønnevåg, Pavel Machek, cgroups-u79uwXL29TY76Z2rM5mHXA On Mon, Apr 29, 2013 at 03:08:31PM -0700, Tejun Heo wrote: > > spent freezing by a factor of 5. It will have a similar effect on a > > non-Android system, although those generally don't care about > > suspend/resume optimization. > > Yeah, if it's something which makes actual difference rather than > "this seems to be a good idea" thing, sure, let's find a way. And a probably better approach would be rolling should_skip test into freeze_task() with a comment explaining the interlocking rather than adding should_skip test in its callers. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set 2013-04-29 22:16 ` Tejun Heo @ 2013-04-30 11:48 ` Rafael J. Wysocki 0 siblings, 0 replies; 7+ messages in thread From: Rafael J. Wysocki @ 2013-04-30 11:48 UTC (permalink / raw) To: Tejun Heo Cc: Colin Cross, Linux PM list, lkml, Arve Hjønnevåg, Li Zefan, Len Brown, Pavel Machek, containers, cgroups On Monday, April 29, 2013 03:16:24 PM Tejun Heo wrote: > On Mon, Apr 29, 2013 at 03:08:31PM -0700, Tejun Heo wrote: > > > spent freezing by a factor of 5. It will have a similar effect on a > > > non-Android system, although those generally don't care about > > > suspend/resume optimization. > > > > Yeah, if it's something which makes actual difference rather than > > "this seems to be a good idea" thing, sure, let's find a way. > > And a probably better approach would be rolling should_skip test into > freeze_task() with a comment explaining the interlocking rather than > adding should_skip test in its callers. Agreed. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-30 11:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1367271946-7239-1-git-send-email-ccross@android.com>
[not found] ` <1367271946-7239-1-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
2013-04-29 21:45 ` [PATCH 02/10] freezer: skip waking up tasks with PF_FREEZER_SKIP set Colin Cross
[not found] ` <1367271946-7239-3-git-send-email-ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
2013-04-29 21:51 ` Tejun Heo
[not found] ` <20130429215157.GA2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-04-29 21:57 ` Tejun Heo
2013-04-29 22:02 ` Colin Cross
[not found] ` <CAMbhsRQHkm=XTYtummEZq1h4sa-tRGwLpk8Uyhpj9D3+jQF3dg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-04-29 22:08 ` Tejun Heo
[not found] ` <20130429220831.GC2395-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-04-29 22:16 ` Tejun Heo
2013-04-30 11:48 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox