linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: rjw@sisk.pl, oleg@redhat.com
Cc: linux-kernel@vger.kernel.org, lizefan@huawei.com,
	containers@lists.linux-foundation.org, cgroups@vger.kernel.org,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 3/7] cgroup_freezer: make it official that writes to freezer.state don't fail
Date: Tue, 16 Oct 2012 15:28:42 -0700	[thread overview]
Message-ID: <1350426526-14254-4-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1350426526-14254-1-git-send-email-tj@kernel.org>

try_to_freeze_cgroup() has condition checks which are intended to fail
the write operation to freezer.state if there are tasks which can't be
frozen.  The condition checks have been broken for quite some time
now.  freeze_task() returns %false if the target task can't be frozen,
so num_cant_freeze_now is never incremented.

In addition, strangely, cgroup freezing proceeds even after the write
is failed, which is rather broken.

This patch rips out the non-working code intended to fail the write to
freezer.state when the cgroup contains non-freezable tasks and makes
it official that writes to freezer.state succeed whether there are
non-freezable tasks in the cgroup or not.

This leaves is_task_frozen_enough() with only one user -
upste_if_frozen().  Collapse it into the caller.  Note that this
removes an extra call to freezing().

This doesn't cause any userland behavior changes.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/cgroup_freezer.c |   43 +++++++++++--------------------------------
 1 files changed, 11 insertions(+), 32 deletions(-)

diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 12bfedb..05d5218 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -150,13 +150,6 @@ static void freezer_destroy(struct cgroup *cgroup)
 	kfree(freezer);
 }
 
-/* task is frozen or will freeze immediately when next it gets woken */
-static bool is_task_frozen_enough(struct task_struct *task)
-{
-	return frozen(task) ||
-		(task_is_stopped_or_traced(task) && freezing(task));
-}
-
 /*
  * The call to cgroup_lock() in the freezer.state write method prevents
  * a write to that file racing against an attach, and hence the
@@ -222,7 +215,8 @@ static void update_if_frozen(struct cgroup *cgroup,
 	cgroup_iter_start(cgroup, &it);
 	while ((task = cgroup_iter_next(cgroup, &it))) {
 		ntotal++;
-		if (freezing(task) && is_task_frozen_enough(task))
+		if (freezing(task) && (frozen(task) ||
+				       task_is_stopped_or_traced(task)))
 			nfrozen++;
 	}
 
@@ -264,24 +258,15 @@ static int freezer_read(struct cgroup *cgroup, struct cftype *cft,
 	return 0;
 }
 
-static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
+static void freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
 {
 	struct cgroup_iter it;
 	struct task_struct *task;
-	unsigned int num_cant_freeze_now = 0;
 
 	cgroup_iter_start(cgroup, &it);
-	while ((task = cgroup_iter_next(cgroup, &it))) {
-		if (!freeze_task(task))
-			continue;
-		if (is_task_frozen_enough(task))
-			continue;
-		if (!freezing(task) && !freezer_should_skip(task))
-			num_cant_freeze_now++;
-	}
+	while ((task = cgroup_iter_next(cgroup, &it)))
+		freeze_task(task);
 	cgroup_iter_end(cgroup, &it);
-
-	return num_cant_freeze_now ? -EBUSY : 0;
 }
 
 static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
@@ -295,13 +280,10 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
 	cgroup_iter_end(cgroup, &it);
 }
 
-static int freezer_change_state(struct cgroup *cgroup,
-				enum freezer_state goal_state)
+static void freezer_change_state(struct cgroup *cgroup,
+				 enum freezer_state goal_state)
 {
-	struct freezer *freezer;
-	int retval = 0;
-
-	freezer = cgroup_freezer(cgroup);
+	struct freezer *freezer = cgroup_freezer(cgroup);
 
 	spin_lock_irq(&freezer->lock);
 
@@ -318,22 +300,19 @@ static int freezer_change_state(struct cgroup *cgroup,
 		if (freezer->state == CGROUP_THAWED)
 			atomic_inc(&system_freezing_cnt);
 		freezer->state = CGROUP_FREEZING;
-		retval = try_to_freeze_cgroup(cgroup, freezer);
+		freeze_cgroup(cgroup, freezer);
 		break;
 	default:
 		BUG();
 	}
 
 	spin_unlock_irq(&freezer->lock);
-
-	return retval;
 }
 
 static int freezer_write(struct cgroup *cgroup,
 			 struct cftype *cft,
 			 const char *buffer)
 {
-	int retval;
 	enum freezer_state goal_state;
 
 	if (strcmp(buffer, freezer_state_strs[CGROUP_THAWED]) == 0)
@@ -345,9 +324,9 @@ static int freezer_write(struct cgroup *cgroup,
 
 	if (!cgroup_lock_live_group(cgroup))
 		return -ENODEV;
-	retval = freezer_change_state(cgroup, goal_state);
+	freezer_change_state(cgroup, goal_state);
 	cgroup_unlock();
-	return retval;
+	return 0;
 }
 
 static struct cftype files[] = {
-- 
1.7.7.3


  parent reply	other threads:[~2012-10-16 22:29 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-16 22:28 [PATCHSET cgroup/for-3.8] cgroup_freezer: allow migration regardless of freezer state and update locking Tejun Heo
2012-10-16 22:28 ` [PATCH 1/7] cgroup: cgroup_subsys->fork() should be called after the task is added to css_set Tejun Heo
2012-10-17  8:28   ` Li Zefan
2012-10-18  1:25     ` Li Zefan
2012-10-21 19:11   ` Oleg Nesterov
2012-10-21 19:22     ` Tejun Heo
2012-10-22 18:04       ` Oleg Nesterov
2012-10-22 21:16         ` Tejun Heo
2012-10-23 15:51           ` Oleg Nesterov
2012-10-24 19:04             ` Tejun Heo
2012-10-25 17:42               ` Oleg Nesterov
2012-12-20  5:25   ` Herton Ronaldo Krzesinski
2012-12-28 21:22     ` [PATCH] cgroup: remove unused dummy cgroup_fork_callbacks() Tejun Heo
2012-10-16 22:28 ` [PATCH 2/7] freezer: add missing mb's to freezer_count() and freezer_should_skip() Tejun Heo
2012-10-22 17:44   ` Oleg Nesterov
2012-10-22 21:13     ` Tejun Heo
2012-10-23 15:39       ` Oleg Nesterov
2012-10-24 18:57         ` Tejun Heo
2012-10-25 16:39           ` [PATCH 0/1] (Was: freezer: add missing mb's to freezer_count() and freezer_should_skip()) Oleg Nesterov
2012-10-25 16:39             ` [PATCH 1/1] freezer: change ptrace_stop/do_signal_stop to use freezable_schedule() Oleg Nesterov
2012-10-25 17:18               ` Tejun Heo
2012-10-25 17:34                 ` Oleg Nesterov
2012-10-25 17:36                   ` Tejun Heo
2012-10-26 17:45                     ` [PATCH v2 0/1] " Oleg Nesterov
2012-10-26 17:46                       ` [PATCH v2 1/1] " Oleg Nesterov
2012-10-26 17:52                         ` Tejun Heo
2012-10-26 18:01                           ` Oleg Nesterov
2012-10-26 21:14                             ` Rafael J. Wysocki
2012-10-26 21:29                               ` Rafael J. Wysocki
2012-10-26 21:29                                 ` Tejun Heo
2012-10-28  0:16                                   ` Rafael J. Wysocki
2012-10-27 22:22                         ` Ben Hutchings
2012-10-28 13:45                           ` Oleg Nesterov
2012-10-16 22:28 ` Tejun Heo [this message]
2012-10-16 22:28 ` [PATCH 4/7] cgroup_freezer: don't stall transition to FROZEN for PF_NOFREEZE or PF_FREEZER_SKIP tasks Tejun Heo
2012-10-22 18:34   ` Oleg Nesterov
2012-10-22 21:18     ` Tejun Heo
2012-10-23 15:55       ` Oleg Nesterov
2012-10-24 19:06         ` Tejun Heo
2012-10-25 17:12           ` [PATCH 0/1] (Was: cgroup_freezer: don't stall transition to FROZEN for PF_NOFREEZE or PF_FREEZER_SKIP tasks) Oleg Nesterov
2012-10-25 17:12             ` [PATCH 1/1] freezer: exec should clear PF_NOFREEZE along with PF_KTHREAD Oleg Nesterov
2012-10-25 17:20               ` Tejun Heo
2012-10-25 17:37                 ` Oleg Nesterov
2012-10-25 17:37                   ` Tejun Heo
2012-10-25 20:13                     ` Rafael J. Wysocki
2012-10-16 22:28 ` [PATCH 5/7] cgroup_freezer: allow moving tasks in and out of a frozen cgroup Tejun Heo
2012-10-22 19:25   ` Oleg Nesterov
2012-10-22 21:25     ` Tejun Heo
2012-10-23 16:14       ` Oleg Nesterov
2012-10-16 22:28 ` [PATCH 6/7] cgroup_freezer: prepare update_if_frozen() for locking change Tejun Heo
2012-10-16 22:28 ` [PATCH 7/7] cgroup_freezer: don't use cgroup_lock_live_group() Tejun Heo
2012-10-17 19:16 ` [PATCHSET cgroup/for-3.8] cgroup_freezer: allow migration regardless of freezer state and update locking Matt Helsley
2012-10-18 21:14   ` Tejun Heo
2012-10-18 22:21     ` Matt Helsley
2012-10-18 22:35       ` Tejun Heo
2012-10-18 23:47         ` Matt Helsley
2012-10-19  0:01           ` Tejun Heo
2012-10-19  1:29             ` Matt Helsley
2012-10-19 20:02               ` Tejun Heo
2012-10-19 16:54 ` Rafael J. Wysocki
2012-10-19 20:04   ` Tejun Heo
2012-10-21 19:18     ` Oleg Nesterov
2012-10-21 19:24       ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1350426526-14254-4-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=oleg@redhat.com \
    --cc=rjw@sisk.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).