memcg: cat: memory.memsw.* : Operation not supported

memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

hi, all

when I used memory cgroup in latest mainline, the following error occurred:

# mount -t cgroup -o memory xxx /cgroup/
# ll /cgroup/memory.memsw.*
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
-r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
# cat /cgroup/memory.memsw.*
cat: /cgroup/memory.memsw.failcnt: Operation not supported
cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported

I'm confusing why it can't read memory.memsw.* files.

as commit:a42c390cfa0c said, CGROUP_MEM_RES_CTLR_SWAP_ENABLED and
swapaccount kernel parameter control memcg swap accounting,
but I confirmed the two options all don't be set:

# cat /usr/lib/modules/3.5.0-rc4+/source/.config | grep CGROUP_MEM
CONFIG_CGROUP_MEM_RES_CTLR=y
CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y
# CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is not set
CONFIG_CGROUP_MEM_RES_CTLR_KMEM=y
# cat /proc/cmdline 
BOOT_IMAGE=/vmlinuz-3.5.0-rc4+ root=/dev/mapper/vg_amd--pike--06-lv_root ro rd.lvm.lv=vg_amd-pike-06/lv_swap rd.md=0 LANG=en_US.UTF-8 console=ttyS0,115200n81 KEYTABLE=us SYSFONT=True rd.luks=0 rd.dm=0 rd.lvm.lv=vg_amd-pike-06/lv_root

so I have two problems here:
 1. when kernel neither set 'CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED' nor 'swapaccount' options,
    why memcg have memory.memsw.* files ?

 2. why we can't read memory.memsw.* ?

Addition info:
when I open CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED option, the above issues are gone.
also I tested v3.4.0, there aren't the two issues, so please take a look.

-- 
Thanks,
Zhouping

memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

hi, all

when I used memory cgroup in latest mainline, the following error occurred:

# mount -t cgroup -o memory xxx /cgroup/
# ll /cgroup/memory.memsw.*
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
-rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
-r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
# cat /cgroup/memory.memsw.*
cat: /cgroup/memory.memsw.failcnt: Operation not supported
cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported

I'm confusing why it can't read memory.memsw.* files.

as commit:a42c390cfa0c said, CGROUP_MEM_RES_CTLR_SWAP_ENABLED and
swapaccount kernel parameter control memcg swap accounting,
but I confirmed the two options all don't be set:

# cat /usr/lib/modules/3.5.0-rc4+/source/.config | grep CGROUP_MEM
CONFIG_CGROUP_MEM_RES_CTLR=y
CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y
# CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is not set
CONFIG_CGROUP_MEM_RES_CTLR_KMEM=y
# cat /proc/cmdline 
BOOT_IMAGE=/vmlinuz-3.5.0-rc4+ root=/dev/mapper/vg_amd--pike--06-lv_root ro rd.lvm.lv=vg_amd-pike-06/lv_swap rd.md=0 LANG=en_US.UTF-8 console=ttyS0,115200n81 KEYTABLE=us SYSFONT=True rd.luks=0 rd.dm=0 rd.lvm.lv=vg_amd-pike-06/lv_root

so I have two problems here:
 1. when kernel neither set 'CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED' nor 'swapaccount' options,
    why memcg have memory.memsw.* files ?

 2. why we can't read memory.memsw.* ?

Addition info:
when I open CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED option, the above issues are gone.
also I tested v3.4.0, there aren't the two issues, so please take a look.

-- 
Thanks,
Zhouping

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Tue 26-06-12 23:49:15, Zhouping Liu wrote:
> hi, all
> 
> when I used memory cgroup in latest mainline, the following error occurred:
> 
> # mount -t cgroup -o memory xxx /cgroup/
> # ll /cgroup/memory.memsw.*
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> # cat /cgroup/memory.memsw.*
> cat: /cgroup/memory.memsw.failcnt: Operation not supported
> cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> 
> I'm confusing why it can't read memory.memsw.* files.

Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
if the feature is turned off when any attempt to open the file returns
EOPNOTSUPP which is exactly what you are seeing.
This is a deliberate decision see: b6d9270d (memcg: always create memsw
files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).

Does this help to explain your problem? Do you actually see any problem
with this behavior?

Thanks
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Tue 26-06-12 23:49:15, Zhouping Liu wrote:
> hi, all
> 
> when I used memory cgroup in latest mainline, the following error occurred:
> 
> # mount -t cgroup -o memory xxx /cgroup/
> # ll /cgroup/memory.memsw.*
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> # cat /cgroup/memory.memsw.*
> cat: /cgroup/memory.memsw.failcnt: Operation not supported
> cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> 
> I'm confusing why it can't read memory.memsw.* files.

Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
if the feature is turned off when any attempt to open the file returns
EOPNOTSUPP which is exactly what you are seeing.
This is a deliberate decision see: b6d9270d (memcg: always create memsw
files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).

Does this help to explain your problem? Do you actually see any problem
with this behavior?

Thanks
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Michal Hocko wrote:

> > # mount -t cgroup -o memory xxx /cgroup/
> > # ll /cgroup/memory.memsw.*
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> > -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> > # cat /cgroup/memory.memsw.*
> > cat: /cgroup/memory.memsw.failcnt: Operation not supported
> > cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> > cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> > cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> > 
> > I'm confusing why it can't read memory.memsw.* files.
> 
> Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
> if the feature is turned off when any attempt to open the file returns
> EOPNOTSUPP which is exactly what you are seeing.
> This is a deliberate decision see: b6d9270d (memcg: always create memsw
> files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).
> 

You mean af36f906c0f4?

> Does this help to explain your problem? Do you actually see any problem
> with this behavior?
> 

I think it's a crappy solution and one that is undocumented in 
Documentation/cgroups/memory.txt.  If you can only enable swap accounting 
at boot either via .config or the command line then these files should 
never be added for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when 
do_swap_account is 0.  It's much easier to test if the feature is enabled 
by checking for the presence of these files at the memcg mount point 
rather than doing an open(2) and checking for -EOPNOTSUPP, which isn't 
even a listed error code.  I don't care how much cleaner it makes the 
internal memcg code.

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Michal Hocko wrote:

> > # mount -t cgroup -o memory xxx /cgroup/
> > # ll /cgroup/memory.memsw.*
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> > -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> > # cat /cgroup/memory.memsw.*
> > cat: /cgroup/memory.memsw.failcnt: Operation not supported
> > cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> > cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> > cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> > 
> > I'm confusing why it can't read memory.memsw.* files.
> 
> Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
> if the feature is turned off when any attempt to open the file returns
> EOPNOTSUPP which is exactly what you are seeing.
> This is a deliberate decision see: b6d9270d (memcg: always create memsw
> files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).
> 

You mean af36f906c0f4?

> Does this help to explain your problem? Do you actually see any problem
> with this behavior?
> 

I think it's a crappy solution and one that is undocumented in 
Documentation/cgroups/memory.txt.  If you can only enable swap accounting 
at boot either via .config or the command line then these files should 
never be added for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when 
do_swap_account is 0.  It's much easier to test if the feature is enabled 
by checking for the presence of these files at the memcg mount point 
rather than doing an open(2) and checking for -EOPNOTSUPP, which isn't 
even a listed error code.  I don't care how much cleaner it makes the 
internal memcg code.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, Michal, David.

On Wed, Jun 27, 2012 at 01:04:51PM -0700, David Rientjes wrote:
> I think it's a crappy solution and one that is undocumented in 
> Documentation/cgroups/memory.txt.  If you can only enable swap accounting 
> at boot either via .config or the command line then these files should 
> never be added for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when 
> do_swap_account is 0.  It's much easier to test if the feature is enabled 
> by checking for the presence of these files at the memcg mount point 
> rather than doing an open(2) and checking for -EOPNOTSUPP, which isn't 
> even a listed error code.  I don't care how much cleaner it makes the 
> internal memcg code.

Yeah, it's kinda ugly.  Taking a step back, do we really need be able
to configure out memsw?  How much vmlinux bloat or runtime overhead
are we talking about?  I don't think config options need to be this
granular.

Thanks.

-- 
tejun

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, Michal, David.

On Wed, Jun 27, 2012 at 01:04:51PM -0700, David Rientjes wrote:
> I think it's a crappy solution and one that is undocumented in 
> Documentation/cgroups/memory.txt.  If you can only enable swap accounting 
> at boot either via .config or the command line then these files should 
> never be added for CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when 
> do_swap_account is 0.  It's much easier to test if the feature is enabled 
> by checking for the presence of these files at the memcg mount point 
> rather than doing an open(2) and checking for -EOPNOTSUPP, which isn't 
> even a listed error code.  I don't care how much cleaner it makes the 
> internal memcg code.

Yeah, it's kinda ugly.  Taking a step back, do we really need be able
to configure out memsw?  How much vmlinux bloat or runtime overhead
are we talking about?  I don't think config options need to be this
granular.

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Tejun Heo wrote:

> Yeah, it's kinda ugly.  Taking a step back, do we really need be able
> to configure out memsw?  How much vmlinux bloat or runtime overhead
> are we talking about?  I don't think config options need to be this
> granular.
> 

Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so 
even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into 
CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP 
since configuring them would imply there is some limit to be enforced.

But to answer your question:

   text	   data	    bss	    dec	    hex	filename
  25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
  27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled

Is it really too painful to not create these files when 
CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow 
them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is 
written?

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Tejun Heo wrote:

> Yeah, it's kinda ugly.  Taking a step back, do we really need be able
> to configure out memsw?  How much vmlinux bloat or runtime overhead
> are we talking about?  I don't think config options need to be this
> granular.
> 

Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so 
even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into 
CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP 
since configuring them would imply there is some limit to be enforced.

But to answer your question:

   text	   data	    bss	    dec	    hex	filename
  25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
  27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled

Is it really too painful to not create these files when 
CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow 
them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is 
written?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello,

On Wed, Jun 27, 2012 at 01:21:27PM -0700, David Rientjes wrote:
> Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so 

Right.

> even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into 
> CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP 
> since configuring them would imply there is some limit to be enforced.
> 
> But to answer your question:
> 
>    text	   data	    bss	    dec	    hex	filename
>   25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
>   27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled

I still wish it's folded into CONFIG_MEMCG and conditionalized just on
CONFIG_SWAP tho.

> Is it really too painful to not create these files when 
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow 
> them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is 
> written?

Not at all, that was the first version anyway, which (IIRC) KAME
didn't like and suggested always creating those files.  KAME, what do
you think?

Thanks.

-- 
tejun

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello,

On Wed, Jun 27, 2012 at 01:21:27PM -0700, David Rientjes wrote:
> Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so 

Right.

> even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into 
> CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP 
> since configuring them would imply there is some limit to be enforced.
> 
> But to answer your question:
> 
>    text	   data	    bss	    dec	    hex	filename
>   25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
>   27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled

I still wish it's folded into CONFIG_MEMCG and conditionalized just on
CONFIG_SWAP tho.

> Is it really too painful to not create these files when 
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow 
> them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is 
> written?

Not at all, that was the first version anyway, which (IIRC) KAME
didn't like and suggested always creating those files.  KAME, what do
you think?

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Tejun Heo wrote:

> >    text	   data	    bss	    dec	    hex	filename
> >   25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
> >   27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled
> 
> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> CONFIG_SWAP tho.
> 

Agreed.

Re: memcg: cat: memory.memsw.* : Operation not supported

[David Rientjes]

On Wed, 27 Jun 2012, Tejun Heo wrote:

> >    text	   data	    bss	    dec	    hex	filename
> >   25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
> >   27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled
> 
> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> CONFIG_SWAP tho.
> 

Agreed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/28 5:24), Tejun Heo wrote:
> Hello,
>
> On Wed, Jun 27, 2012 at 01:21:27PM -0700, David Rientjes wrote:
>> Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so
>
> Right.
>
>> even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into
>> CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP
>> since configuring them would imply there is some limit to be enforced.
>>
>> But to answer your question:
>>
>>     text	   data	    bss	    dec	    hex	filename
>>    25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
>>    27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled
>
> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> CONFIG_SWAP tho.
>

In old days, memsw controller was not very stable. So, we devided the config.
And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
That is the problem.

>> Is it really too painful to not create these files when
>> CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow
>> them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is
>> written?
>
> Not at all, that was the first version anyway, which (IIRC) KAME
> didn't like and suggested always creating those files.  KAME, what do
> you think?
>

IIRC...at that time, we made decision, cgroup has no feature to
'create files dynamically'. Then, we made it in static, decision was done
at compile time and ignores "do_swap_account".

Now, IIUC, we have the feature. So, it's may be a time to create the file
with regard to "do_swap_account", making decision at boot time.


Thanks,
-Kame

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/28 5:24), Tejun Heo wrote:
> Hello,
>
> On Wed, Jun 27, 2012 at 01:21:27PM -0700, David Rientjes wrote:
>> Well it also has a prerequisite that memcg doesn't have: CONFIG_SWAP, so
>
> Right.
>
>> even if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is folded into
>> CONFIG_CGROUP_MEM_RES_CTLR, then these should still depend on CONFIG_SWAP
>> since configuring them would imply there is some limit to be enforced.
>>
>> But to answer your question:
>>
>>     text	   data	    bss	    dec	    hex	filename
>>    25777	   3644	   4128	  33549	   830d	memcontrol.o.swap_disabled
>>    27294	   4476	   4128	  35898	   8c3a	memcontrol.o.swap_enabled
>
> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> CONFIG_SWAP tho.
>

In old days, memsw controller was not very stable. So, we devided the config.
And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
That is the problem.

>> Is it really too painful to not create these files when
>> CONFIG_CGROUP_MEM_RES_CTLR_SWAP is disabled?  If so, can we at least allow
>> them to be opened but return -EINVAL if memory.memsw.limit_in_bytes is
>> written?
>
> Not at all, that was the first version anyway, which (IIRC) KAME
> didn't like and suggested always creating those files.  KAME, what do
> you think?
>

IIRC...at that time, we made decision, cgroup has no feature to
'create files dynamically'. Then, we made it in static, decision was done
at compile time and ignores "do_swap_account".

Now, IIUC, we have the feature. So, it's may be a time to create the file
with regard to "do_swap_account", making decision at boot time.


Thanks,
-Kame



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

[Adding Kame and Aneesh to CC]

On Wed 27-06-12 13:04:51, David Rientjes wrote:
> On Wed, 27 Jun 2012, Michal Hocko wrote:
> 
> > > # mount -t cgroup -o memory xxx /cgroup/
> > > # ll /cgroup/memory.memsw.*
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> > > -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> > > # cat /cgroup/memory.memsw.*
> > > cat: /cgroup/memory.memsw.failcnt: Operation not supported
> > > cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> > > cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> > > cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> > > 
> > > I'm confusing why it can't read memory.memsw.* files.
> > 
> > Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
> > if the feature is turned off when any attempt to open the file returns
> > EOPNOTSUPP which is exactly what you are seeing.
> > This is a deliberate decision see: b6d9270d (memcg: always create memsw
> > files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).
> > 
> 
> You mean af36f906c0f4?

Ahh, right. The other one was from the mm tree. Sorry about the confusion.

> > Does this help to explain your problem? Do you actually see any problem
> > with this behavior?
> > 
> 
> I think it's a crappy solution and one that is undocumented in 
> Documentation/cgroups/memory.txt.  

Yes the documentation part is really missing. I don't think the current
state is ideal as well...

> If you can only enable swap accounting at boot either via .config     
> or the command line then these files should never be added for        
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when do_swap_account is 0.       

Yes, I think we can enhance the internal implementation to support
configurable files (hugetlb controler would benefit from it as well
because the exported files depend on the supported/configured huge page
sizes). What about something like (totally untested) patch bellow? If
this sounds like a reasonable thing to support I can spin a regular
patch...
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index d3f5fba..3fc7859 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -527,6 +527,7 @@ struct cgroup_subsys {
 
 	/* base cftypes, automatically [de]registered with subsys itself */
 	struct cftype *base_cftypes;
+	bool (*cftype_enabled)(const char *name);
 	struct cftype_set base_cftset;
 
 	/* should be defined only by modular subsystems */
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 0f3527d..0d1a25d 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
 	int err, ret = 0;
 
 	for (cft = cfts; cft->name[0] != '\0'; cft++) {
+		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
+			continue;
+
 		if (is_add)
 			err = cgroup_add_file(cgrp, subsys, cft);
 		else
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a2677e0..45b65ba 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -72,6 +72,13 @@ static int really_do_swap_account __initdata = 1;
 static int really_do_swap_account __initdata = 0;
 #endif
 
+bool mem_cgroup_file_enabled(const char *name)
+{
+	if (!strncmp("memsw.", name, 6))
+		return do_swap_account;
+	return true;
+}
+
 #else
 #define do_swap_account		0
 #endif
@@ -5521,6 +5528,9 @@ struct cgroup_subsys mem_cgroup_subsys = {
 	.cancel_attach = mem_cgroup_cancel_attach,
 	.attach = mem_cgroup_move_task,
 	.base_cftypes = mem_cgroup_files,
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
+	.cftype_enabled = mem_cgroup_file_enabled,
+#endif
 	.early_init = 0,
 	.use_id = 1,
 	.__DEPRECATED_clear_css_refs = true,

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

[Adding Kame and Aneesh to CC]

On Wed 27-06-12 13:04:51, David Rientjes wrote:
> On Wed, 27 Jun 2012, Michal Hocko wrote:
> 
> > > # mount -t cgroup -o memory xxx /cgroup/
> > > # ll /cgroup/memory.memsw.*
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.failcnt
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.limit_in_bytes
> > > -rw-r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.max_usage_in_bytes
> > > -r--r--r--. 1 root root 0 Jun 26 23:17 /cgroup/memory.memsw.usage_in_bytes
> > > # cat /cgroup/memory.memsw.*
> > > cat: /cgroup/memory.memsw.failcnt: Operation not supported
> > > cat: /cgroup/memory.memsw.limit_in_bytes: Operation not supported
> > > cat: /cgroup/memory.memsw.max_usage_in_bytes: Operation not supported
> > > cat: /cgroup/memory.memsw.usage_in_bytes: Operation not supported
> > > 
> > > I'm confusing why it can't read memory.memsw.* files.
> > 
> > Those files are exported if CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y even
> > if the feature is turned off when any attempt to open the file returns
> > EOPNOTSUPP which is exactly what you are seeing.
> > This is a deliberate decision see: b6d9270d (memcg: always create memsw
> > files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP).
> > 
> 
> You mean af36f906c0f4?

Ahh, right. The other one was from the mm tree. Sorry about the confusion.

> > Does this help to explain your problem? Do you actually see any problem
> > with this behavior?
> > 
> 
> I think it's a crappy solution and one that is undocumented in 
> Documentation/cgroups/memory.txt.  

Yes the documentation part is really missing. I don't think the current
state is ideal as well...

> If you can only enable swap accounting at boot either via .config     
> or the command line then these files should never be added for        
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP=n or when do_swap_account is 0.       

Yes, I think we can enhance the internal implementation to support
configurable files (hugetlb controler would benefit from it as well
because the exported files depend on the supported/configured huge page
sizes). What about something like (totally untested) patch bellow? If
this sounds like a reasonable thing to support I can spin a regular
patch...
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index d3f5fba..3fc7859 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -527,6 +527,7 @@ struct cgroup_subsys {
 
 	/* base cftypes, automatically [de]registered with subsys itself */
 	struct cftype *base_cftypes;
+	bool (*cftype_enabled)(const char *name);
 	struct cftype_set base_cftset;
 
 	/* should be defined only by modular subsystems */
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 0f3527d..0d1a25d 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
 	int err, ret = 0;
 
 	for (cft = cfts; cft->name[0] != '\0'; cft++) {
+		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
+			continue;
+
 		if (is_add)
 			err = cgroup_add_file(cgrp, subsys, cft);
 		else
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a2677e0..45b65ba 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -72,6 +72,13 @@ static int really_do_swap_account __initdata = 1;
 static int really_do_swap_account __initdata = 0;
 #endif
 
+bool mem_cgroup_file_enabled(const char *name)
+{
+	if (!strncmp("memsw.", name, 6))
+		return do_swap_account;
+	return true;
+}
+
 #else
 #define do_swap_account		0
 #endif
@@ -5521,6 +5528,9 @@ struct cgroup_subsys mem_cgroup_subsys = {
 	.cancel_attach = mem_cgroup_cancel_attach,
 	.attach = mem_cgroup_move_task,
 	.base_cftypes = mem_cgroup_files,
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
+	.cftype_enabled = mem_cgroup_file_enabled,
+#endif
 	.early_init = 0,
 	.use_id = 1,
 	.__DEPRECATED_clear_css_refs = true,

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, Michal.

On Thu, Jun 28, 2012 at 02:36:11PM +0200, Michal Hocko wrote:
> @@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
>  	int err, ret = 0;
>  
>  	for (cft = cfts; cft->name[0] != '\0'; cft++) {
> +		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
> +			continue;
> +
>  		if (is_add)
>  			err = cgroup_add_file(cgrp, subsys, cft);
>  		else

I hope we could avoid this dynamic decision.  That was one of the main
reasons behind doing the cftype thing.  It's better to be able to
"declare" these kind of things rather than being able to implement
fully flexible dynamic logic.  Too much flexibility often doesn't
achieve much while being a hindrance to evolution of code base (trying
to improve / simplify X - ooh... there's this single wacko corner case
YYY here which is really different from all other users).

really_do_swap_account can't change once booted, right?  Why not just
separate out memsw cfts into a separate array and call
cgroup_add_cftypes() from init path?  Can't we do that from
enable_swap_cgroup()?

Thanks.

-- 
tejun

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, Michal.

On Thu, Jun 28, 2012 at 02:36:11PM +0200, Michal Hocko wrote:
> @@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
>  	int err, ret = 0;
>  
>  	for (cft = cfts; cft->name[0] != '\0'; cft++) {
> +		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
> +			continue;
> +
>  		if (is_add)
>  			err = cgroup_add_file(cgrp, subsys, cft);
>  		else

I hope we could avoid this dynamic decision.  That was one of the main
reasons behind doing the cftype thing.  It's better to be able to
"declare" these kind of things rather than being able to implement
fully flexible dynamic logic.  Too much flexibility often doesn't
achieve much while being a hindrance to evolution of code base (trying
to improve / simplify X - ooh... there's this single wacko corner case
YYY here which is really different from all other users).

really_do_swap_account can't change once booted, right?  Why not just
separate out memsw cfts into a separate array and call
cgroup_add_cftypes() from init path?  Can't we do that from
enable_swap_cgroup()?

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, KAME.

On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> >I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> >CONFIG_SWAP tho.
> >
> 
> In old days, memsw controller was not very stable. So, we devided the config.
> And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
> That is the problem.

I see.  Do you think it's now reasonable to drop the separate config
option?  Having memcg enabled but swap unaccounted sounds half-broken
to me.

> IIRC...at that time, we made decision, cgroup has no feature to
> 'create files dynamically'. Then, we made it in static, decision was done
> at compile time and ignores "do_swap_account".
> 
> Now, IIUC, we have the feature. So, it's may be a time to create the file
> with regard to "do_swap_account", making decision at boot time.

Heh, yeah, maybe I'm confused about how it happened.  Anyways, let's
get it fixed.

Thanks!

-- 
tejun

Re: memcg: cat: memory.memsw.* : Operation not supported

[Tejun Heo]

Hello, KAME.

On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> >I still wish it's folded into CONFIG_MEMCG and conditionalized just on
> >CONFIG_SWAP tho.
> >
> 
> In old days, memsw controller was not very stable. So, we devided the config.
> And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
> That is the problem.

I see.  Do you think it's now reasonable to drop the separate config
option?  Having memcg enabled but swap unaccounted sounds half-broken
to me.

> IIRC...at that time, we made decision, cgroup has no feature to
> 'create files dynamically'. Then, we made it in static, decision was done
> at compile time and ignores "do_swap_account".
> 
> Now, IIUC, we have the feature. So, it's may be a time to create the file
> with regard to "do_swap_account", making decision at boot time.

Heh, yeah, maybe I'm confused about how it happened.  Anyways, let's
get it fixed.

Thanks!

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/29 3:29), Tejun Heo wrote:
> Hello, Michal.
>
> On Thu, Jun 28, 2012 at 02:36:11PM +0200, Michal Hocko wrote:
>> @@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
>>   	int err, ret = 0;
>>
>>   	for (cft = cfts; cft->name[0] != '\0'; cft++) {
>> +		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
>> +			continue;
>> +
>>   		if (is_add)
>>   			err = cgroup_add_file(cgrp, subsys, cft);
>>   		else
>
> I hope we could avoid this dynamic decision.  That was one of the main
> reasons behind doing the cftype thing.  It's better to be able to
> "declare" these kind of things rather than being able to implement
> fully flexible dynamic logic.  Too much flexibility often doesn't
> achieve much while being a hindrance to evolution of code base (trying
> to improve / simplify X - ooh... there's this single wacko corner case
> YYY here which is really different from all other users).
>
> really_do_swap_account can't change once booted, right?  Why not just
> separate out memsw cfts into a separate array and call
> cgroup_add_cftypes() from init path?  Can't we do that from
> enable_swap_cgroup()?
>

Yes, that's will be good.

Thanks,
-Kame

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/29 3:29), Tejun Heo wrote:
> Hello, Michal.
>
> On Thu, Jun 28, 2012 at 02:36:11PM +0200, Michal Hocko wrote:
>> @@ -2726,6 +2726,9 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys,
>>   	int err, ret = 0;
>>
>>   	for (cft = cfts; cft->name[0] != '\0'; cft++) {
>> +		if (subsys->cftype_enabled && !subsys->cftype_enabled(cft->name))
>> +			continue;
>> +
>>   		if (is_add)
>>   			err = cgroup_add_file(cgrp, subsys, cft);
>>   		else
>
> I hope we could avoid this dynamic decision.  That was one of the main
> reasons behind doing the cftype thing.  It's better to be able to
> "declare" these kind of things rather than being able to implement
> fully flexible dynamic logic.  Too much flexibility often doesn't
> achieve much while being a hindrance to evolution of code base (trying
> to improve / simplify X - ooh... there's this single wacko corner case
> YYY here which is really different from all other users).
>
> really_do_swap_account can't change once booted, right?  Why not just
> separate out memsw cfts into a separate array and call
> cgroup_add_cftypes() from init path?  Can't we do that from
> enable_swap_cgroup()?
>

Yes, that's will be good.

Thanks,
-Kame


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Glauber Costa]

On 06/28/2012 08:04 AM, Kamezawa Hiroyuki wrote:
>>
>> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
>> CONFIG_SWAP tho.
>>
> 
> In old days, memsw controller was not very stable. So, we devided the
> config.
> And, it makes size of memory for swap-device double (adds 2bytes per
> swapent.)
> That is the problem.


That's the tendency to happen with anything new, since we want to add it
without disrupting what's already in there. I am not very fond of config
options explosions myself, so I am for removing it.

Re: memcg: cat: memory.memsw.* : Operation not supported

[Glauber Costa]

On 06/28/2012 08:04 AM, Kamezawa Hiroyuki wrote:
>>
>> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
>> CONFIG_SWAP tho.
>>
> 
> In old days, memsw controller was not very stable. So, we devided the
> config.
> And, it makes size of memory for swap-device double (adds 2bytes per
> swapent.)
> That is the problem.


That's the tendency to happen with anything new, since we want to add it
without disrupting what's already in there. I am not very fond of config
options explosions myself, so I am for removing it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/29 3:31), Tejun Heo wrote:
> Hello, KAME.
>
> On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
>>> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
>>> CONFIG_SWAP tho.
>>>
>>
>> In old days, memsw controller was not very stable. So, we devided the config.
>> And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
>> That is the problem.
>
> I see.  Do you think it's now reasonable to drop the separate config
> option?  Having memcg enabled but swap unaccounted sounds half-broken
> to me.
>

Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in the next week.

Thanks,
-Kame

Re: memcg: cat: memory.memsw.* : Operation not supported

[Kamezawa Hiroyuki]

(2012/06/29 3:31), Tejun Heo wrote:
> Hello, KAME.
>
> On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
>>> I still wish it's folded into CONFIG_MEMCG and conditionalized just on
>>> CONFIG_SWAP tho.
>>>
>>
>> In old days, memsw controller was not very stable. So, we devided the config.
>> And, it makes size of memory for swap-device double (adds 2bytes per swapent.)
>> That is the problem.
>
> I see.  Do you think it's now reasonable to drop the separate config
> option?  Having memcg enabled but swap unaccounted sounds half-broken
> to me.
>

Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in the next week.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

----- Original Message -----
> From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
> To: "Tejun Heo" <tj@kernel.org>
> Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
> linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
> <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
> Sent: Saturday, June 30, 2012 11:45:41 AM
> Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
> 
> (2012/06/29 3:31), Tejun Heo wrote:
> > Hello, KAME.
> >
> > On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> >>> I still wish it's folded into CONFIG_MEMCG and conditionalized
> >>> just on
> >>> CONFIG_SWAP tho.
> >>>
> >>
> >> In old days, memsw controller was not very stable. So, we devided
> >> the config.
> >> And, it makes size of memory for swap-device double (adds 2bytes
> >> per swapent.)
> >> That is the problem.
> >
> > I see.  Do you think it's now reasonable to drop the separate
> > config
> > option?  Having memcg enabled but swap unaccounted sounds
> > half-broken
> > to me.
> >
> 
> Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
> the next week.

Hello Kame and All,

Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)

do you have any updates for the issue?

I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:

[root@dhcp-8-128 ~] cat .config  | grep -i memcg
CONFIG_MEMCG=y
CONFIG_MEMCG_SWAP=y
# CONFIG_MEMCG_SWAP_ENABLED is not set
CONFIG_MEMCG_KMEM=y
[root@dhcp-8-128 ~] uname -r
3.8.0-rc4+
[root@dhcp-8-128 ~] cat memory.memsw.*
cat: memory.memsw.failcnt: Operation not supported
cat: memory.memsw.limit_in_bytes: Operation not supported
cat: memory.memsw.max_usage_in_bytes: Operation not supported
cat: memory.memsw.usage_in_bytes: Operation not supported

As David said, we should not export memory.memsw.* files if we disable CONFIG_MEMCG_SWAP_ENABLED, or return -EINVAL, right?
(please correct me if I'm wrong)

-- 
Thanks,
Zhouping

Re: memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

----- Original Message -----
> From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
> To: "Tejun Heo" <tj@kernel.org>
> Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
> linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
> <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
> Sent: Saturday, June 30, 2012 11:45:41 AM
> Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
> 
> (2012/06/29 3:31), Tejun Heo wrote:
> > Hello, KAME.
> >
> > On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> >>> I still wish it's folded into CONFIG_MEMCG and conditionalized
> >>> just on
> >>> CONFIG_SWAP tho.
> >>>
> >>
> >> In old days, memsw controller was not very stable. So, we devided
> >> the config.
> >> And, it makes size of memory for swap-device double (adds 2bytes
> >> per swapent.)
> >> That is the problem.
> >
> > I see.  Do you think it's now reasonable to drop the separate
> > config
> > option?  Having memcg enabled but swap unaccounted sounds
> > half-broken
> > to me.
> >
> 
> Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
> the next week.

Hello Kame and All,

Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)

do you have any updates for the issue?

I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:

[root@dhcp-8-128 ~] cat .config  | grep -i memcg
CONFIG_MEMCG=y
CONFIG_MEMCG_SWAP=y
# CONFIG_MEMCG_SWAP_ENABLED is not set
CONFIG_MEMCG_KMEM=y
[root@dhcp-8-128 ~] uname -r
3.8.0-rc4+
[root@dhcp-8-128 ~] cat memory.memsw.*
cat: memory.memsw.failcnt: Operation not supported
cat: memory.memsw.limit_in_bytes: Operation not supported
cat: memory.memsw.max_usage_in_bytes: Operation not supported
cat: memory.memsw.usage_in_bytes: Operation not supported

As David said, we should not export memory.memsw.* files if we disable CONFIG_MEMCG_SWAP_ENABLED, or return -EINVAL, right?
(please correct me if I'm wrong)

-- 
Thanks,
Zhouping

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Mon 21-01-13 03:39:07, Zhouping Liu wrote:
> 
> 
> ----- Original Message -----
> > From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
> > To: "Tejun Heo" <tj@kernel.org>
> > Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
> > linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
> > <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
> > Sent: Saturday, June 30, 2012 11:45:41 AM
> > Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
> > 
> > (2012/06/29 3:31), Tejun Heo wrote:
> > > Hello, KAME.
> > >
> > > On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> > >>> I still wish it's folded into CONFIG_MEMCG and conditionalized
> > >>> just on
> > >>> CONFIG_SWAP tho.
> > >>>
> > >>
> > >> In old days, memsw controller was not very stable. So, we devided
> > >> the config.
> > >> And, it makes size of memory for swap-device double (adds 2bytes
> > >> per swapent.)
> > >> That is the problem.
> > >
> > > I see.  Do you think it's now reasonable to drop the separate
> > > config
> > > option?  Having memcg enabled but swap unaccounted sounds
> > > half-broken
> > > to me.
> > >
> > 
> > Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
> > the next week.
> 
> Hello Kame and All,
> 
> Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)
> 
> do you have any updates for the issue?
> 
> I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:
> 
> [root@dhcp-8-128 ~] cat .config  | grep -i memcg
> CONFIG_MEMCG=y
> CONFIG_MEMCG_SWAP=y
> # CONFIG_MEMCG_SWAP_ENABLED is not set
> CONFIG_MEMCG_KMEM=y
> [root@dhcp-8-128 ~] uname -r
> 3.8.0-rc4+
> [root@dhcp-8-128 ~] cat memory.memsw.*
> cat: memory.memsw.failcnt: Operation not supported
> cat: memory.memsw.limit_in_bytes: Operation not supported
> cat: memory.memsw.max_usage_in_bytes: Operation not supported
> cat: memory.memsw.usage_in_bytes: Operation not supported

Ohh, this one got lost. I thought Kame was working on that.
Anyway the patch bellow should work:
---
>From 5f8141bf7d27014cfbc7b450f13f6146b5ab099d Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Mon, 21 Jan 2013 11:33:26 +0100
Subject: [PATCH] memcg: Do not create memsw files if swap accounting is
 disabled

Zhouping Liu has reported that memsw files are exported even though
swap accounting is runtime disabled if CONFIG_MEMCG_SWAP is enabled.
This behavior has been introduced by af36f906 (memcg: always create
memsw files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP) and it causes any
attempt to open the file to return EOPNOTSUPP. Although EOPNOTSUPP
should say be clear that memsw operations are not supported in the given
configuration it is fair to say that this behavior could be quite
confusing.

Let's tear memsw files out of default cgroup files and add
them only if the swap accounting is really enabled (either by
CONFIG_MEMCG_SWAP_ENABLED or swapaccount=1 boot parameter). We can
hook into mem_cgroup_init which is called when the memcg subsystem is
initialized and which happens after boot command line is processed.

Reported-by: Zhouping Liu <zliu@redhat.com>
Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |   94 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3817460..a2800db 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5988,33 +5988,6 @@ static struct cftype mem_cgroup_files[] = {
 		.read_seq_string = memcg_numa_stat_show,
 	},
 #endif
-#ifdef CONFIG_MEMCG_SWAP
-	{
-		.name = "memsw.usage_in_bytes",
-		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_USAGE),
-		.read = mem_cgroup_read,
-		.register_event = mem_cgroup_usage_register_event,
-		.unregister_event = mem_cgroup_usage_unregister_event,
-	},
-	{
-		.name = "memsw.max_usage_in_bytes",
-		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_MAX_USAGE),
-		.trigger = mem_cgroup_reset,
-		.read = mem_cgroup_read,
-	},
-	{
-		.name = "memsw.limit_in_bytes",
-		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_LIMIT),
-		.write_string = mem_cgroup_write,
-		.read = mem_cgroup_read,
-	},
-	{
-		.name = "memsw.failcnt",
-		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_FAILCNT),
-		.trigger = mem_cgroup_reset,
-		.read = mem_cgroup_read,
-	},
-#endif
 #ifdef CONFIG_MEMCG_KMEM
 	{
 		.name = "kmem.limit_in_bytes",
@@ -6057,6 +6030,36 @@ static struct cftype mem_cgroup_files[] = {
 	{ },	/* terminate */
 };
 
+#ifdef CONFIG_MEMCG_SWAP
+static struct cftype memsw_cgroup_files[] = {
+	{
+		.name = "memsw.usage_in_bytes",
+		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_USAGE),
+		.read = mem_cgroup_read,
+		.register_event = mem_cgroup_usage_register_event,
+		.unregister_event = mem_cgroup_usage_unregister_event,
+	},
+	{
+		.name = "memsw.max_usage_in_bytes",
+		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_MAX_USAGE),
+		.trigger = mem_cgroup_reset,
+		.read = mem_cgroup_read,
+	},
+	{
+		.name = "memsw.limit_in_bytes",
+		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_LIMIT),
+		.write_string = mem_cgroup_write,
+		.read = mem_cgroup_read,
+	},
+	{
+		.name = "memsw.failcnt",
+		.private = MEMFILE_PRIVATE(_MEMSWAP, RES_FAILCNT),
+		.trigger = mem_cgroup_reset,
+		.read = mem_cgroup_read,
+	},
+	{ },	/* terminate */
+};
+#endif
 static int alloc_mem_cgroup_per_zone_info(struct mem_cgroup *memcg, int node)
 {
 	struct mem_cgroup_per_node *pn;
@@ -6959,19 +6962,6 @@ struct cgroup_subsys mem_cgroup_subsys = {
 	.use_id = 1,
 };
 
-/*
- * The rest of init is performed during ->css_alloc() for root css which
- * happens before initcalls.  hotcpu_notifier() can't be done together as
- * it would introduce circular locking by adding cgroup_lock -> cpu hotplug
- * dependency.  Do it from a subsys_initcall().
- */
-static int __init mem_cgroup_init(void)
-{
-	hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
-	return 0;
-}
-subsys_initcall(mem_cgroup_init);
-
 #ifdef CONFIG_MEMCG_SWAP
 static int __init enable_swap_account(char *s)
 {
@@ -6984,4 +6974,28 @@ static int __init enable_swap_account(char *s)
 }
 __setup("swapaccount=", enable_swap_account);
 
+static void __init memsw_file_init(void)
+{
+	if (really_do_swap_account)
+		WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys,
+					memsw_cgroup_files));
+}
+#else
+static void __init memsw_file_init(void)
+{
+}
 #endif
+
+/*
+ * The rest of init is performed during ->css_alloc() for root css which
+ * happens before initcalls.  hotcpu_notifier() can't be done together as
+ * it would introduce circular locking by adding cgroup_lock -> cpu hotplug
+ * dependency.  Do it from a subsys_initcall().
+ */
+static int __init mem_cgroup_init(void)
+{
+	hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
+	memsw_file_init();
+	return 0;
+}
+subsys_initcall(mem_cgroup_init);
-- 
1.7.10.4

-- 
Michal Hocko
SUSE Labs

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Mon 21-01-13 03:39:07, Zhouping Liu wrote:
> 
> 
> ----- Original Message -----
> > From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
> > To: "Tejun Heo" <tj@kernel.org>
> > Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
> > linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
> > <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
> > Sent: Saturday, June 30, 2012 11:45:41 AM
> > Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
> > 
> > (2012/06/29 3:31), Tejun Heo wrote:
> > > Hello, KAME.
> > >
> > > On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
> > >>> I still wish it's folded into CONFIG_MEMCG and conditionalized
> > >>> just on
> > >>> CONFIG_SWAP tho.
> > >>>
> > >>
> > >> In old days, memsw controller was not very stable. So, we devided
> > >> the config.
> > >> And, it makes size of memory for swap-device double (adds 2bytes
> > >> per swapent.)
> > >> That is the problem.
> > >
> > > I see.  Do you think it's now reasonable to drop the separate
> > > config
> > > option?  Having memcg enabled but swap unaccounted sounds
> > > half-broken
> > > to me.
> > >
> > 
> > Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
> > the next week.
> 
> Hello Kame and All,
> 
> Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)
> 
> do you have any updates for the issue?
> 
> I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
> CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:
> 
> [root@dhcp-8-128 ~] cat .config  | grep -i memcg
> CONFIG_MEMCG=y
> CONFIG_MEMCG_SWAP=y
> # CONFIG_MEMCG_SWAP_ENABLED is not set
> CONFIG_MEMCG_KMEM=y
> [root@dhcp-8-128 ~] uname -r
> 3.8.0-rc4+
> [root@dhcp-8-128 ~] cat memory.memsw.*
> cat: memory.memsw.failcnt: Operation not supported
> cat: memory.memsw.limit_in_bytes: Operation not supported
> cat: memory.memsw.max_usage_in_bytes: Operation not supported
> cat: memory.memsw.usage_in_bytes: Operation not supported

Ohh, this one got lost. I thought Kame was working on that.
Anyway the patch bellow should work:
---

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

And we can put another clean up patch on top of the fix:
---
>From a81d36002e7dbd976b617d5eadf32265f36038a4 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Mon, 21 Jan 2013 12:05:29 +0100
Subject: [PATCH] memcg: clean up swap accounting initialization code

Memcg swap accounting is currently enabled by enable_swap_cgroup when
the root cgroup is created. mem_cgroup_init acts as a memcg subsystem
initializer which sounds like a much better place for enable_swap_cgroup
as well. We already register memsw files from there so it makes a lot of
sense to merge those two into a single enable_swap_cgroup function.

This patch doesn't introduce any semantical changes.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |   30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a2800db..878d62c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6221,18 +6221,6 @@ struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg)
 }
 EXPORT_SYMBOL(parent_mem_cgroup);
 
-#ifdef CONFIG_MEMCG_SWAP
-static void __init enable_swap_cgroup(void)
-{
-	if (!mem_cgroup_disabled() && really_do_swap_account)
-		do_swap_account = 1;
-}
-#else
-static void __init enable_swap_cgroup(void)
-{
-}
-#endif
-
 static int mem_cgroup_soft_limit_tree_init(void)
 {
 	struct mem_cgroup_tree_per_node *rtpn;
@@ -6286,7 +6274,6 @@ mem_cgroup_css_alloc(struct cgroup *cont)
 	/* root ? */
 	if (cont->parent == NULL) {
 		int cpu;
-		enable_swap_cgroup();
 		parent = NULL;
 		if (mem_cgroup_soft_limit_tree_init())
 			goto free_out;
@@ -6976,12 +6963,19 @@ __setup("swapaccount=", enable_swap_account);
 
 static void __init memsw_file_init(void)
 {
-	if (really_do_swap_account)
-		WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys,
-					memsw_cgroup_files));
+	WARN_ON(cgroup_add_cftypes(&mem_cgroup_subsys, memsw_cgroup_files));
+}
+
+static void __init enable_swap_cgroup(void)
+{
+	if (!mem_cgroup_disabled() && really_do_swap_account) {
+		do_swap_account = 1;
+		memsw_file_init();
+	}
 }
+
 #else
-static void __init memsw_file_init(void)
+static void __init enable_swap_cgroup(void)
 {
 }
 #endif
@@ -6995,7 +6989,7 @@ static void __init memsw_file_init(void)
 static int __init mem_cgroup_init(void)
 {
 	hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
-	memsw_file_init();
+	enable_swap_cgroup();
 	return 0;
 }
 subsys_initcall(mem_cgroup_init);
-- 
1.7.10.4

-- 
Michal Hocko
SUSE Labs

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

And we can put another clean up patch on top of the fix:
---

Re: memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

On 01/21/2013 06:56 PM, Michal Hocko wrote:
> On Mon 21-01-13 03:39:07, Zhouping Liu wrote:
>>
>> ----- Original Message -----
>>> From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
>>> To: "Tejun Heo" <tj@kernel.org>
>>> Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
>>> linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
>>> <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
>>> Sent: Saturday, June 30, 2012 11:45:41 AM
>>> Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
>>>
>>> (2012/06/29 3:31), Tejun Heo wrote:
>>>> Hello, KAME.
>>>>
>>>> On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
>>>>>> I still wish it's folded into CONFIG_MEMCG and conditionalized
>>>>>> just on
>>>>>> CONFIG_SWAP tho.
>>>>>>
>>>>> In old days, memsw controller was not very stable. So, we devided
>>>>> the config.
>>>>> And, it makes size of memory for swap-device double (adds 2bytes
>>>>> per swapent.)
>>>>> That is the problem.
>>>> I see.  Do you think it's now reasonable to drop the separate
>>>> config
>>>> option?  Having memcg enabled but swap unaccounted sounds
>>>> half-broken
>>>> to me.
>>>>
>>> Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
>>> the next week.
>> Hello Kame and All,
>>
>> Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)
>>
>> do you have any updates for the issue?
>>
>> I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
>> CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:
>>
>> [root@dhcp-8-128 ~] cat .config  | grep -i memcg
>> CONFIG_MEMCG=y
>> CONFIG_MEMCG_SWAP=y
>> # CONFIG_MEMCG_SWAP_ENABLED is not set
>> CONFIG_MEMCG_KMEM=y
>> [root@dhcp-8-128 ~] uname -r
>> 3.8.0-rc4+
>> [root@dhcp-8-128 ~] cat memory.memsw.*
>> cat: memory.memsw.failcnt: Operation not supported
>> cat: memory.memsw.limit_in_bytes: Operation not supported
>> cat: memory.memsw.max_usage_in_bytes: Operation not supported
>> cat: memory.memsw.usage_in_bytes: Operation not supported
> Ohh, this one got lost. I thought Kame was working on that.
> Anyway the patch bellow should work:
> ---
>  From 5f8141bf7d27014cfbc7b450f13f6146b5ab099d Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Mon, 21 Jan 2013 11:33:26 +0100
> Subject: [PATCH] memcg: Do not create memsw files if swap accounting is
>   disabled
>
> Zhouping Liu has reported that memsw files are exported even though
> swap accounting is runtime disabled if CONFIG_MEMCG_SWAP is enabled.
> This behavior has been introduced by af36f906 (memcg: always create
> memsw files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP) and it causes any
> attempt to open the file to return EOPNOTSUPP. Although EOPNOTSUPP
> should say be clear that memsw operations are not supported in the given
> configuration it is fair to say that this behavior could be quite
> confusing.
>
> Let's tear memsw files out of default cgroup files and add
> them only if the swap accounting is really enabled (either by
> CONFIG_MEMCG_SWAP_ENABLED or swapaccount=1 boot parameter). We can
> hook into mem_cgroup_init which is called when the memcg subsystem is
> initialized and which happens after boot command line is processed.

Thanks for your quick patch, your patch looks good for me.

I tested it with or without CONFIG_MEMCG_SWAP_ENABLED=y,
and also tested it with swapaccount=1 kernel parameters, all are okay.

Tested-by: Zhouping Liu <zliu@redhat.com>

Thanks,
Zhouping

Re: memcg: cat: memory.memsw.* : Operation not supported

[Zhouping Liu]

On 01/21/2013 06:56 PM, Michal Hocko wrote:
> On Mon 21-01-13 03:39:07, Zhouping Liu wrote:
>>
>> ----- Original Message -----
>>> From: "Kamezawa Hiroyuki" <kamezawa.hiroyu@jp.fujitsu.com>
>>> To: "Tejun Heo" <tj@kernel.org>
>>> Cc: "David Rientjes" <rientjes@google.com>, "Michal Hocko" <mhocko@suse.cz>, "Zhouping Liu" <zliu@redhat.com>,
>>> linux-mm@kvack.org, "Li Zefan" <lizefan@huawei.com>, "CAI Qian" <caiqian@redhat.com>, "LKML"
>>> <linux-kernel@vger.kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>
>>> Sent: Saturday, June 30, 2012 11:45:41 AM
>>> Subject: Re: memcg: cat: memory.memsw.* : Operation not supported
>>>
>>> (2012/06/29 3:31), Tejun Heo wrote:
>>>> Hello, KAME.
>>>>
>>>> On Thu, Jun 28, 2012 at 01:04:16PM +0900, Kamezawa Hiroyuki wrote:
>>>>>> I still wish it's folded into CONFIG_MEMCG and conditionalized
>>>>>> just on
>>>>>> CONFIG_SWAP tho.
>>>>>>
>>>>> In old days, memsw controller was not very stable. So, we devided
>>>>> the config.
>>>>> And, it makes size of memory for swap-device double (adds 2bytes
>>>>> per swapent.)
>>>>> That is the problem.
>>>> I see.  Do you think it's now reasonable to drop the separate
>>>> config
>>>> option?  Having memcg enabled but swap unaccounted sounds
>>>> half-broken
>>>> to me.
>>>>
>>> Hmm. Maybe it's ok if we can keep boot option. I'll cook a patch in
>>> the next week.
>> Hello Kame and All,
>>
>> Sorry for so delay to open the thread. (please open the link https://lkml.org/lkml/2012/6/26/547 if you don't remember the topic)
>>
>> do you have any updates for the issue?
>>
>> I checked the latest version, if we don't open CONFIG_MEMCG_SWAP_ENABLED(commit c255a458055e changed
>> CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED as CONFIG_MEMCG_SWAP_ENABLED), the issue still exist:
>>
>> [root@dhcp-8-128 ~] cat .config  | grep -i memcg
>> CONFIG_MEMCG=y
>> CONFIG_MEMCG_SWAP=y
>> # CONFIG_MEMCG_SWAP_ENABLED is not set
>> CONFIG_MEMCG_KMEM=y
>> [root@dhcp-8-128 ~] uname -r
>> 3.8.0-rc4+
>> [root@dhcp-8-128 ~] cat memory.memsw.*
>> cat: memory.memsw.failcnt: Operation not supported
>> cat: memory.memsw.limit_in_bytes: Operation not supported
>> cat: memory.memsw.max_usage_in_bytes: Operation not supported
>> cat: memory.memsw.usage_in_bytes: Operation not supported
> Ohh, this one got lost. I thought Kame was working on that.
> Anyway the patch bellow should work:
> ---
>  From 5f8141bf7d27014cfbc7b450f13f6146b5ab099d Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Mon, 21 Jan 2013 11:33:26 +0100
> Subject: [PATCH] memcg: Do not create memsw files if swap accounting is
>   disabled
>
> Zhouping Liu has reported that memsw files are exported even though
> swap accounting is runtime disabled if CONFIG_MEMCG_SWAP is enabled.
> This behavior has been introduced by af36f906 (memcg: always create
> memsw files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP) and it causes any
> attempt to open the file to return EOPNOTSUPP. Although EOPNOTSUPP
> should say be clear that memsw operations are not supported in the given
> configuration it is fair to say that this behavior could be quite
> confusing.
>
> Let's tear memsw files out of default cgroup files and add
> them only if the swap accounting is really enabled (either by
> CONFIG_MEMCG_SWAP_ENABLED or swapaccount=1 boot parameter). We can
> hook into mem_cgroup_init which is called when the memcg subsystem is
> initialized and which happens after boot command line is processed.

Thanks for your quick patch, your patch looks good for me.

I tested it with or without CONFIG_MEMCG_SWAP_ENABLED=y,
and also tested it with swapaccount=1 kernel parameters, all are okay.

Tested-by: Zhouping Liu <zliu@redhat.com>

Thanks,
Zhouping

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Mon 21-01-13 21:27:33, Zhouping Liu wrote:
> On 01/21/2013 06:56 PM, Michal Hocko wrote:
[...]
> > From 5f8141bf7d27014cfbc7b450f13f6146b5ab099d Mon Sep 17 00:00:00 2001
> >From: Michal Hocko <mhocko@suse.cz>
> >Date: Mon, 21 Jan 2013 11:33:26 +0100
> >Subject: [PATCH] memcg: Do not create memsw files if swap accounting is
> >  disabled
> >
> >Zhouping Liu has reported that memsw files are exported even though
> >swap accounting is runtime disabled if CONFIG_MEMCG_SWAP is enabled.
> >This behavior has been introduced by af36f906 (memcg: always create
> >memsw files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP) and it causes any
> >attempt to open the file to return EOPNOTSUPP. Although EOPNOTSUPP
> >should say be clear that memsw operations are not supported in the given
> >configuration it is fair to say that this behavior could be quite
> >confusing.
> >
> >Let's tear memsw files out of default cgroup files and add
> >them only if the swap accounting is really enabled (either by
> >CONFIG_MEMCG_SWAP_ENABLED or swapaccount=1 boot parameter). We can
> >hook into mem_cgroup_init which is called when the memcg subsystem is
> >initialized and which happens after boot command line is processed.
> 
> Thanks for your quick patch, your patch looks good for me.
> 
> I tested it with or without CONFIG_MEMCG_SWAP_ENABLED=y,
> and also tested it with swapaccount=1 kernel parameters, all are okay.
> 
> Tested-by: Zhouping Liu <zliu@redhat.com>

Thanks for testing!

-- 
Michal Hocko
SUSE Labs

Re: memcg: cat: memory.memsw.* : Operation not supported

[Michal Hocko]

On Mon 21-01-13 21:27:33, Zhouping Liu wrote:
> On 01/21/2013 06:56 PM, Michal Hocko wrote:
[...]
> > From 5f8141bf7d27014cfbc7b450f13f6146b5ab099d Mon Sep 17 00:00:00 2001
> >From: Michal Hocko <mhocko@suse.cz>
> >Date: Mon, 21 Jan 2013 11:33:26 +0100
> >Subject: [PATCH] memcg: Do not create memsw files if swap accounting is
> >  disabled
> >
> >Zhouping Liu has reported that memsw files are exported even though
> >swap accounting is runtime disabled if CONFIG_MEMCG_SWAP is enabled.
> >This behavior has been introduced by af36f906 (memcg: always create
> >memsw files if CONFIG_CGROUP_MEM_RES_CTLR_SWAP) and it causes any
> >attempt to open the file to return EOPNOTSUPP. Although EOPNOTSUPP
> >should say be clear that memsw operations are not supported in the given
> >configuration it is fair to say that this behavior could be quite
> >confusing.
> >
> >Let's tear memsw files out of default cgroup files and add
> >them only if the swap accounting is really enabled (either by
> >CONFIG_MEMCG_SWAP_ENABLED or swapaccount=1 boot parameter). We can
> >hook into mem_cgroup_init which is called when the memcg subsystem is
> >initialized and which happens after boot command line is processed.
> 
> Thanks for your quick patch, your patch looks good for me.
> 
> I tested it with or without CONFIG_MEMCG_SWAP_ENABLED=y,
> and also tested it with swapaccount=1 kernel parameters, all are okay.
> 
> Tested-by: Zhouping Liu <zliu@redhat.com>

Thanks for testing!

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>