[PATCH 0/2] ResCounter: Add res_counter_read_uint and use it in memory cgroup

[PATCH 0/2] ResCounter: Add res_counter_read_uint and use it in memory cgroup

[menage]

These patches simplify the code required to report values from a
res_counter object in a cgroups control file.

The first patch adds res_counter_read_uint, which simply reports the
current value for a res_counter member.

The second replaces the existing mem_cgroup_read() with a simpler
version that calls res_counter_read_uint().

Signed-off-by: Paul Menage <menage@google.com>

--

[PATCH 1/2] ResCounter: Add res_counter_read_uint()

[menage]

[-- Attachment #1: resource_counter_read_uint.patch --]
[-- Type: text/plain, Size: 1670 bytes --]

Adds a function for returning the value of a resource counter member,
in a form suitable for use in a cgroup read_uint control file method.

Signed-off-by: Paul Menage <menage@google.com>

---
 include/linux/res_counter.h |    1 +
 kernel/res_counter.c        |    5 +++++
 2 files changed, 6 insertions(+)

Index: rescounter-2.6.25-rc2-mm1/include/linux/res_counter.h
===================================================================
--- rescounter-2.6.25-rc2-mm1.orig/include/linux/res_counter.h
+++ rescounter-2.6.25-rc2-mm1/include/linux/res_counter.h
@@ -54,6 +54,7 @@ struct res_counter {
 ssize_t res_counter_read(struct res_counter *counter, int member,
 		const char __user *buf, size_t nbytes, loff_t *pos,
 		int (*read_strategy)(unsigned long long val, char *s));
+u64 res_counter_read_uint(struct res_counter *counter, int member);
 ssize_t res_counter_write(struct res_counter *counter, int member,
 		const char __user *buf, size_t nbytes, loff_t *pos,
 		int (*write_strategy)(char *buf, unsigned long long *val));
Index: rescounter-2.6.25-rc2-mm1/kernel/res_counter.c
===================================================================
--- rescounter-2.6.25-rc2-mm1.orig/kernel/res_counter.c
+++ rescounter-2.6.25-rc2-mm1/kernel/res_counter.c
@@ -92,6 +92,11 @@ ssize_t res_counter_read(struct res_coun
 			pos, buf, s - buf);
 }
 
+u64 res_counter_read_uint(struct res_counter *counter, int member)
+{
+	return *res_counter_member(counter, member);
+}
+
 ssize_t res_counter_write(struct res_counter *counter, int member,
 		const char __user *userbuf, size_t nbytes, loff_t *pos,
 		int (*write_strategy)(char *st_buf, unsigned long long *val))

--

[PATCH 2/2] ResCounter: Use read_uint in memory controller

[menage]

[-- Attachment #1: memcontrol_use_res_counter_read_uint.patch --]
[-- Type: text/plain, Size: 1578 bytes --]

Update the memory controller to use read_uint for its
limit/usage/failcnt control files, calling the new
res_counter_read_uint() function.

Signed-off-by: Paul Menage <menage@google.com>

---
 mm/memcontrol.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Index: rescounter-2.6.25-rc2-mm1/mm/memcontrol.c
===================================================================
--- rescounter-2.6.25-rc2-mm1.orig/mm/memcontrol.c
+++ rescounter-2.6.25-rc2-mm1/mm/memcontrol.c
@@ -922,13 +922,10 @@ int mem_cgroup_write_strategy(char *buf,
 	return 0;
 }
 
-static ssize_t mem_cgroup_read(struct cgroup *cont,
-			struct cftype *cft, struct file *file,
-			char __user *userbuf, size_t nbytes, loff_t *ppos)
+static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
 {
-	return res_counter_read(&mem_cgroup_from_cont(cont)->res,
-				cft->private, userbuf, nbytes, ppos,
-				NULL);
+	return res_counter_read_uint(&mem_cgroup_from_cont(cont)->res,
+				     cft->private);
 }
 
 static ssize_t mem_cgroup_write(struct cgroup *cont, struct cftype *cft,
@@ -1024,18 +1021,18 @@ static struct cftype mem_cgroup_files[] 
 	{
 		.name = "usage_in_bytes",
 		.private = RES_USAGE,
-		.read = mem_cgroup_read,
+		.read_uint = mem_cgroup_read,
 	},
 	{
 		.name = "limit_in_bytes",
 		.private = RES_LIMIT,
 		.write = mem_cgroup_write,
-		.read = mem_cgroup_read,
+		.read_uint = mem_cgroup_read,
 	},
 	{
 		.name = "failcnt",
 		.private = RES_FAILCNT,
-		.read = mem_cgroup_read,
+		.read_uint = mem_cgroup_read,
 	},
 	{
 		.name = "force_empty",

--

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Balbir Singh]

menage@google.com wrote:
> Update the memory controller to use read_uint for its
> limit/usage/failcnt control files, calling the new
> res_counter_read_uint() function.
> 
> Signed-off-by: Paul Menage <menage@google.com>
> 

Hi, Paul,

Looks good, except for the name uint(), can we make it u64(). Integers are 32
bit on both ILP32 and LP64, but we really read/write 64 bit values.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Pavel Emelyanov]

menage@google.com wrote:
> Update the memory controller to use read_uint for its
> limit/usage/failcnt control files, calling the new
> res_counter_read_uint() function.
> 
> Signed-off-by: Paul Menage <menage@google.com>

Acked-by: Pavel Emelyanov <xemul@openvz.org>

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Andrew Morton]

On Fri, 22 Feb 2008 09:59:41 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> menage@google.com wrote:
> > Update the memory controller to use read_uint for its
> > limit/usage/failcnt control files, calling the new
> > res_counter_read_uint() function.
> > 
> > Signed-off-by: Paul Menage <menage@google.com>
> > 
> 
> Hi, Paul,
> 
> Looks good, except for the name uint(), can we make it u64(). Integers are 32
> bit on both ILP32 and LP64, but we really read/write 64 bit values.
> 

yup, I agree.  Even though I don't know what ILP32 and LP64 are ;)

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[KOSAKI Motohiro]

Hi Andrew,

> yup, I agree.  Even though I don't know what ILP32 and LP64 are ;)

ILP32: integer and long and pointer size is 32bit
LP64:  long and pointer size is 64bit, but int size is 32bit

linux 32bit kernel obey ILP32 model, 64bit kernel obey LP64.

Thanks.


- kosaki

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Balbir Singh]

Andrew Morton wrote:
> On Fri, 22 Feb 2008 09:59:41 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
>> menage@google.com wrote:
>>> Update the memory controller to use read_uint for its
>>> limit/usage/failcnt control files, calling the new
>>> res_counter_read_uint() function.
>>>
>>> Signed-off-by: Paul Menage <menage@google.com>
>>>
>> Hi, Paul,
>>
>> Looks good, except for the name uint(), can we make it u64(). Integers are 32
>> bit on both ILP32 and LP64, but we really read/write 64 bit values.
>>
> 
> yup, I agree.  Even though I don't know what ILP32 and LP64 are ;)

ILP32 and LP64 are programming models. They stand for Integer, Long, Pointer 32
bit for 32 bit systems and Long, Pointer 64 bit for 64 bit systems (which
implies integers are 32 bit).

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Paul Menage]

On Thu, Feb 21, 2008 at 8:29 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>
>  Looks good, except for the name uint(), can we make it u64(). Integers are 32
>  bit on both ILP32 and LP64, but we really read/write 64 bit values.

Yes, that's true. But read_uint() is more consistent with all the
other instances in cgroups and subsystems. So if we were to call it
res_counter_read_u64() I'd also want to rename all the other
*read_uint functions/fields to *read_u64 too. Can I do that in a
separate patch?

Paul

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Andrew Morton]

On Sat, 23 Feb 2008 06:33:34 -0800 "Paul Menage" <menage@google.com> wrote:

> On Thu, Feb 21, 2008 at 8:29 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> >
> >  Looks good, except for the name uint(), can we make it u64(). Integers are 32
> >  bit on both ILP32 and LP64, but we really read/write 64 bit values.
> 
> Yes, that's true. But read_uint() is more consistent with all the
> other instances in cgroups and subsystems. So if we were to call it
> res_counter_read_u64() I'd also want to rename all the other
> *read_uint functions/fields to *read_u64 too. Can I do that in a
> separate patch?
> 

Sounds sensible to me.

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Balbir Singh]

Andrew Morton wrote:
> On Sat, 23 Feb 2008 06:33:34 -0800 "Paul Menage" <menage@google.com> wrote:
> 
>> On Thu, Feb 21, 2008 at 8:29 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>>>  Looks good, except for the name uint(), can we make it u64(). Integers are 32
>>>  bit on both ILP32 and LP64, but we really read/write 64 bit values.
>> Yes, that's true. But read_uint() is more consistent with all the
>> other instances in cgroups and subsystems. So if we were to call it
>> res_counter_read_u64() I'd also want to rename all the other
>> *read_uint functions/fields to *read_u64 too. Can I do that in a
>> separate patch?
>>
> 
> Sounds sensible to me.
> 

Sure, fair enough.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH 2/2] ResCounter: Use read_uint in memory controller

[Paul Menage]

On Sat, Feb 23, 2008 at 6:47 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>  >> res_counter_read_u64() I'd also want to rename all the other
>  >> *read_uint functions/fields to *read_u64 too. Can I do that in a
>  >> separate patch?
>  >>
>  >
>  > Sounds sensible to me.
>  >
>
>  Sure, fair enough.
>

Actually, since multiple people were asking for this change I did the
search/replace and sent it out already (as a precursor of the other
patches in the series that I sent today).

Paul