[PATCH 0/2] res_counter: Merge some common code

[PATCH 0/2] res_counter: Merge some common code

[Frederic Weisbecker]

Just a small code factorizing. I hope Glauber can have a look at the second
patch to confirm this is what we want for the net memcg controller (that
is the only user of the nofail version AFAICS).

Thanks.

Frederic Weisbecker (2):
  res_counter: Merge res_counter_charge and res_counter_charge_nofail
  res_counter: Account max_usage when calling
    res_counter_charge_nofail()

 Documentation/cgroups/resource_counter.txt |    4 +-
 include/linux/res_counter.h                |    2 +-
 kernel/res_counter.c                       |   71 +++++++++++++---------------
 3 files changed, 36 insertions(+), 41 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] res_counter: Merge res_counter_charge and res_counter_charge_nofail

[Frederic Weisbecker]

These two functions do almost the same thing and duplicate some code.
Merge their implementation into a single common function.
res_counter_charge_locked() takes one more parameter but it doesn't seem
to be used outside res_counter.c yet anyway.

There is no (intended) change in the behaviour.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
---
 Documentation/cgroups/resource_counter.txt |    4 +-
 include/linux/res_counter.h                |    2 +-
 kernel/res_counter.c                       |   73 +++++++++++++---------------
 3 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/Documentation/cgroups/resource_counter.txt b/Documentation/cgroups/resource_counter.txt
index 95b24d7..f3c4ec3 100644
--- a/Documentation/cgroups/resource_counter.txt
+++ b/Documentation/cgroups/resource_counter.txt
@@ -77,11 +77,11 @@ to work with it.
 	where the charging failed.
 
  d. int res_counter_charge_locked
-			(struct res_counter *rc, unsigned long val)
+			(struct res_counter *rc, unsigned long val, bool force)
 
 	The same as res_counter_charge(), but it must not acquire/release the
 	res_counter->lock internally (it must be called with res_counter->lock
-	held).
+	held). The force parameter indicates whether we can bypass the limit.
 
  e. void res_counter_uncharge[_locked]
 			(struct res_counter *rc, unsigned long val)
diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h
index da81af0..fb20189 100644
--- a/include/linux/res_counter.h
+++ b/include/linux/res_counter.h
@@ -116,7 +116,7 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent);
  */
 
 int __must_check res_counter_charge_locked(struct res_counter *counter,
-		unsigned long val);
+					   unsigned long val, bool force);
 int __must_check res_counter_charge(struct res_counter *counter,
 		unsigned long val, struct res_counter **limit_fail_at);
 int __must_check res_counter_charge_nofail(struct res_counter *counter,
diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index d508363..07a2992 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -22,75 +22,70 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent)
 	counter->parent = parent;
 }
 
-int res_counter_charge_locked(struct res_counter *counter, unsigned long val)
+int res_counter_charge_locked(struct res_counter *counter, unsigned long val,
+			      bool force)
 {
+	int ret = 0;
+
 	if (counter->usage + val > counter->limit) {
 		counter->failcnt++;
-		return -ENOMEM;
+		ret = -ENOMEM;
+		if (!force)
+			return ret;
 	}
 
 	counter->usage += val;
-	if (counter->usage > counter->max_usage)
+	if (!force && counter->usage > counter->max_usage)
 		counter->max_usage = counter->usage;
-	return 0;
+	return ret;
 }
 
-int res_counter_charge(struct res_counter *counter, unsigned long val,
-			struct res_counter **limit_fail_at)
+static int __res_counter_charge(struct res_counter *counter, unsigned long val,
+				struct res_counter **limit_fail_at, bool force)
 {
-	int ret;
+	int ret, r;
 	unsigned long flags;
 	struct res_counter *c, *u;
 
+	r = ret = 0;
 	*limit_fail_at = NULL;
 	local_irq_save(flags);
 	for (c = counter; c != NULL; c = c->parent) {
 		spin_lock(&c->lock);
-		ret = res_counter_charge_locked(c, val);
+		r = res_counter_charge_locked(c, val, force);
 		spin_unlock(&c->lock);
-		if (ret < 0) {
+		if (r < 0 && !ret) {
+			ret = r;
 			*limit_fail_at = c;
-			goto undo;
+			if (!force)
+				break;
 		}
 	}
-	ret = 0;
-	goto done;
-undo:
-	for (u = counter; u != c; u = u->parent) {
-		spin_lock(&u->lock);
-		res_counter_uncharge_locked(u, val);
-		spin_unlock(&u->lock);
+
+	if (ret < 0 && !force) {
+		for (u = counter; u != c; u = u->parent) {
+			spin_lock(&u->lock);
+			res_counter_uncharge_locked(u, val);
+			spin_unlock(&u->lock);
+		}
 	}
-done:
 	local_irq_restore(flags);
+
 	return ret;
 }
 
+int res_counter_charge(struct res_counter *counter, unsigned long val,
+			struct res_counter **limit_fail_at)
+{
+	return __res_counter_charge(counter, val, limit_fail_at, false);
+}
+
 int res_counter_charge_nofail(struct res_counter *counter, unsigned long val,
 			      struct res_counter **limit_fail_at)
 {
-	int ret, r;
-	unsigned long flags;
-	struct res_counter *c;
-
-	r = ret = 0;
-	*limit_fail_at = NULL;
-	local_irq_save(flags);
-	for (c = counter; c != NULL; c = c->parent) {
-		spin_lock(&c->lock);
-		r = res_counter_charge_locked(c, val);
-		if (r)
-			c->usage += val;
-		spin_unlock(&c->lock);
-		if (r < 0 && ret == 0) {
-			*limit_fail_at = c;
-			ret = r;
-		}
-	}
-	local_irq_restore(flags);
-
-	return ret;
+	return __res_counter_charge(counter, val, limit_fail_at, true);
 }
+
 void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val)
 {
 	if (WARN_ON(counter->usage < val))
-- 
1.7.5.4

[PATCH 2/2] res_counter: Account max_usage when calling res_counter_charge_nofail()

[Frederic Weisbecker]

Updating max_usage is something one would expect when we reach
a new maximum usage value even when we do this by forcing through
the limit with res_counter_charge_nofail().

(Whether we want to account failcnt when we force through the limit
is another debate).

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
---
 kernel/res_counter.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index 07a2992..bebe2b1 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -35,7 +35,7 @@ int res_counter_charge_locked(struct res_counter *counter, unsigned long val,
 	}
 
 	counter->usage += val;
-	if (!force && counter->usage > counter->max_usage)
+	if (counter->usage > counter->max_usage)
 		counter->max_usage = counter->usage;
 	return ret;
 }
-- 
1.7.5.4

Re: [PATCH 1/2] res_counter: Merge res_counter_charge and res_counter_charge_nofail

[KAMEZAWA Hiroyuki]

(2012/04/25 8:11), Frederic Weisbecker wrote:

> These two functions do almost the same thing and duplicate some code.
> Merge their implementation into a single common function.
> res_counter_charge_locked() takes one more parameter but it doesn't seem
> to be used outside res_counter.c yet anyway.
> 
> There is no (intended) change in the behaviour.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>


Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [PATCH 2/2] res_counter: Account max_usage when calling res_counter_charge_nofail()

[KAMEZAWA Hiroyuki]

(2012/04/25 8:11), Frederic Weisbecker wrote:

> Updating max_usage is something one would expect when we reach
> a new maximum usage value even when we do this by forcing through
> the limit with res_counter_charge_nofail().
> 
> (Whether we want to account failcnt when we force through the limit
> is another debate).
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>


Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [PATCH 0/2] res_counter: Merge some common code

[Kirill A. Shutemov]

On Wed, Apr 25, 2012 at 01:11:34AM +0200, Frederic Weisbecker wrote:
> Just a small code factorizing. I hope Glauber can have a look at the second
> patch to confirm this is what we want for the net memcg controller (that
> is the only user of the nofail version AFAICS).
> 
> Thanks.
> 
> Frederic Weisbecker (2):
>   res_counter: Merge res_counter_charge and res_counter_charge_nofail
>   res_counter: Account max_usage when calling
>     res_counter_charge_nofail()
> 
>  Documentation/cgroups/resource_counter.txt |    4 +-
>  include/linux/res_counter.h                |    2 +-
>  kernel/res_counter.c                       |   71 +++++++++++++---------------
>  3 files changed, 36 insertions(+), 41 deletions(-)

Acked-by: Kirill A. Shutemov <kirill@shutemov.name>

-- 
 Kirill A. Shutemov

Re: [PATCH 1/2] res_counter: Merge res_counter_charge and res_counter_charge_nofail

[Glauber Costa]

On 04/24/2012 09:28 PM, KAMEZAWA Hiroyuki wrote:
> (2012/04/25 8:11), Frederic Weisbecker wrote:
> 
>> These two functions do almost the same thing and duplicate some code.
>> Merge their implementation into a single common function.
>> res_counter_charge_locked() takes one more parameter but it doesn't seem
>> to be used outside res_counter.c yet anyway.
>>
>> There is no (intended) change in the behaviour.
>>
>> Signed-off-by: Frederic Weisbecker<fweisbec@gmail.com>
>> Cc: Tejun Heo<tj@kernel.org>
>> Cc: Li Zefan<lizefan@huawei.com>
>> Cc: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>> Cc: Glauber Costa<glommer@parallels.com>
>> Cc: Kirill A. Shutemov<kirill@shutemov.name>
> 
> 
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> 
Acked-by: Glauber Costa <glommer@parallels.com>

Re: [PATCH 2/2] res_counter: Account max_usage when calling res_counter_charge_nofail()

[Glauber Costa]

On 04/24/2012 09:29 PM, KAMEZAWA Hiroyuki wrote:
> (2012/04/25 8:11), Frederic Weisbecker wrote:
> 
>> Updating max_usage is something one would expect when we reach
>> a new maximum usage value even when we do this by forcing through
>> the limit with res_counter_charge_nofail().
>>
>> (Whether we want to account failcnt when we force through the limit
>> is another debate).
>>
>> Signed-off-by: Frederic Weisbecker<fweisbec@gmail.com>
>> Cc: Tejun Heo<tj@kernel.org>
>> Cc: Li Zefan<lizefan@huawei.com>
>> Cc: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>> Cc: Glauber Costa<glommer@parallels.com>
>> Cc: Kirill A. Shutemov<kirill@shutemov.name>
> 
> 
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
> 
Acked-by: Glauber Costa <glommer@parallels.com>

Re: [PATCH 1/2] res_counter: Merge res_counter_charge and res_counter_charge_nofail

[Tejun Heo]

On Wed, Apr 25, 2012 at 01:11:35AM +0200, Frederic Weisbecker wrote:
> These two functions do almost the same thing and duplicate some code.
> Merge their implementation into a single common function.
> res_counter_charge_locked() takes one more parameter but it doesn't seem
> to be used outside res_counter.c yet anyway.
> 
> There is no (intended) change in the behaviour.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>

Applied to cgroup/for-3.5.  Thanks.

-- 
tejun

Re: [PATCH 2/2] res_counter: Account max_usage when calling res_counter_charge_nofail()

[Tejun Heo]

On Wed, Apr 25, 2012 at 01:11:36AM +0200, Frederic Weisbecker wrote:
> Updating max_usage is something one would expect when we reach
> a new maximum usage value even when we do this by forcing through
> the limit with res_counter_charge_nofail().
> 
> (Whether we want to account failcnt when we force through the limit
> is another debate).
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>

Applied to cgroup/for-3.5.  Thanks.

-- 
tejun