Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 19-05-14 17:50:18, Michal Hocko wrote: > On Mon 19-05-14 16:02:48, Michal Hocko wrote: > > On Fri 16-05-14 15:00:16, Greg Thelen wrote: > [...] > > > -- First, demonstrate that just rmdir, without memory.force_empty, > > >temporarily hides reparented child memory stats. > > > > > > $ /test > > > p/memory.stat:rss 0 > > > p/memory.stat:total_rss 69632 > > > p/c/memory.stat:rss 69632 > > > p/c/memory.stat:total_rss 69632 > > > For a small time the p/c memory has not been reparented to p. > > > p/memory.stat:rss 0 > > > p/memory.stat:total_rss 0 > > > > OK, this is a bug. Our iterators skip the children because css_tryget > > fails on it but css_offline still not done. Recent cgroup changes distinguish css_tryget and css_tryget_online (http://marc.info/?l=linux-kernel=140025648704805). So we will only need to use css_tryget rather than the _online variant in __mem_cgroup_iter_next. I guess this is what Johannes was talking about. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 19-05-14 17:50:18, Michal Hocko wrote: On Mon 19-05-14 16:02:48, Michal Hocko wrote: On Fri 16-05-14 15:00:16, Greg Thelen wrote: [...] -- First, demonstrate that just rmdir, without memory.force_empty, temporarily hides reparented child memory stats. $ /test p/memory.stat:rss 0 p/memory.stat:total_rss 69632 p/c/memory.stat:rss 69632 p/c/memory.stat:total_rss 69632 For a small time the p/c memory has not been reparented to p. p/memory.stat:rss 0 p/memory.stat:total_rss 0 OK, this is a bug. Our iterators skip the children because css_tryget fails on it but css_offline still not done. Recent cgroup changes distinguish css_tryget and css_tryget_online (http://marc.info/?l=linux-kernelm=140025648704805). So we will only need to use css_tryget rather than the _online variant in __mem_cgroup_iter_next. I guess this is what Johannes was talking about. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 19-05-14 16:02:48, Michal Hocko wrote: > On Fri 16-05-14 15:00:16, Greg Thelen wrote: [...] > > -- First, demonstrate that just rmdir, without memory.force_empty, > >temporarily hides reparented child memory stats. > > > > $ /test > > p/memory.stat:rss 0 > > p/memory.stat:total_rss 69632 > > p/c/memory.stat:rss 69632 > > p/c/memory.stat:total_rss 69632 > > For a small time the p/c memory has not been reparented to p. > > p/memory.stat:rss 0 > > p/memory.stat:total_rss 0 > > OK, this is a bug. Our iterators skip the children because css_tryget > fails on it but css_offline still not done. Or use the cgroups iterator directly. Which would be even easier to fix. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Fri 16-05-14 15:00:16, Greg Thelen wrote:
> On Tue, May 13 2014, Michal Hocko wrote:
[...]
> > If somebody really cares because reparented pages, which would be
> > dropped otherwise, push out more important ones then we should fix the
> > reparenting code and put pages to the tail.
>
> I should mention a case where I've needed to use memory.force_empty: to
> synchronously flush stats from child to parent. Without force_empty
> memory.stat is temporarily inconsistent until async css_offline
> reparents charges. Here is an example on v3.14 showing that
> parent/memory.stat contents are in-flux immediately after rmdir of
> parent/child.
OK, it is true that the delayed offlining makes this little bit
complicated because there is no direct user visible relation between
rmdir and css_offline.
> $ cat /test
> #!/bin/bash
>
> # Create parent and child. Add some non-reclaimable anon rss to child,
> # then move running task to parent.
> mkdir p p/c
> (echo $BASHPID > p/c/cgroup.procs && exec sleep 1d) &
> pid=$!
> sleep 1
> echo $pid > p/cgroup.procs
>
> grep 'rss ' {p,p/c}/memory.stat
> if [[ $1 == force ]]; then
> echo 1 > p/c/memory.force_empty
> fi
> rmdir p/c
>
> echo 'For a small time the p/c memory has not been reparented to p.'
> grep 'rss ' {p,p/c}/memory.stat
>
> sleep 1
> echo 'After waiting all memory has been reparented'
> grep 'rss ' {p,p/c}/memory.stat
>
> kill $pid
> rmdir p
>
>
> -- First, demonstrate that just rmdir, without memory.force_empty,
>temporarily hides reparented child memory stats.
>
> $ /test
> p/memory.stat:rss 0
> p/memory.stat:total_rss 69632
> p/c/memory.stat:rss 69632
> p/c/memory.stat:total_rss 69632
> For a small time the p/c memory has not been reparented to p.
> p/memory.stat:rss 0
> p/memory.stat:total_rss 0
OK, this is a bug. Our iterators skip the children because css_tryget
fails on it but css_offline still not done. This is fixable, though,
and force_empty is just a workaround so I wouldn't see this as a proper
justification to keep it alive.
One possible way to fix this is to iterate children even when css_tryget
fails for them if they haven't finished css_offline yet.
There are some changes in the cgroups core which should make this easier
and Johannes claimed he has some work in that area.
Anyway this is a useful testcase. Thanks Greg!
> grep: p/c/memory.stat: No such file or directory
> After waiting all memory has been reparented
> p/memory.stat:rss 69632
> p/memory.stat:total_rss 69632
> grep: p/c/memory.stat: No such file or directory
> /test: Terminated ( echo $BASHPID > p/c/cgroup.procs && exec
> sleep 1d )
>
> -- Demonstrate that using memory.force_empty before rmdir, behaves more
>sensibly. Stats for reparented child memory are not hidden.
>
> $ /test force
> p/memory.stat:rss 0
> p/memory.stat:total_rss 69632
> p/c/memory.stat:rss 69632
> p/c/memory.stat:total_rss 69632
> For a small time the p/c memory has not been reparented to p.
> p/memory.stat:rss 69632
> p/memory.stat:total_rss 69632
> grep: p/c/memory.stat: No such file or directory
> After waiting all memory has been reparented
> p/memory.stat:rss 69632
> p/memory.stat:total_rss 69632
> grep: p/c/memory.stat: No such file or directory
> /test: Terminated ( echo $BASHPID > p/c/cgroup.procs && exec
> sleep 1d )
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Fri 16-05-14 15:00:16, Greg Thelen wrote:
On Tue, May 13 2014, Michal Hocko mho...@suse.cz wrote:
[...]
If somebody really cares because reparented pages, which would be
dropped otherwise, push out more important ones then we should fix the
reparenting code and put pages to the tail.
I should mention a case where I've needed to use memory.force_empty: to
synchronously flush stats from child to parent. Without force_empty
memory.stat is temporarily inconsistent until async css_offline
reparents charges. Here is an example on v3.14 showing that
parent/memory.stat contents are in-flux immediately after rmdir of
parent/child.
OK, it is true that the delayed offlining makes this little bit
complicated because there is no direct user visible relation between
rmdir and css_offline.
$ cat /test
#!/bin/bash
# Create parent and child. Add some non-reclaimable anon rss to child,
# then move running task to parent.
mkdir p p/c
(echo $BASHPID p/c/cgroup.procs exec sleep 1d)
pid=$!
sleep 1
echo $pid p/cgroup.procs
grep 'rss ' {p,p/c}/memory.stat
if [[ $1 == force ]]; then
echo 1 p/c/memory.force_empty
fi
rmdir p/c
echo 'For a small time the p/c memory has not been reparented to p.'
grep 'rss ' {p,p/c}/memory.stat
sleep 1
echo 'After waiting all memory has been reparented'
grep 'rss ' {p,p/c}/memory.stat
kill $pid
rmdir p
-- First, demonstrate that just rmdir, without memory.force_empty,
temporarily hides reparented child memory stats.
$ /test
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 0
p/memory.stat:total_rss 0
OK, this is a bug. Our iterators skip the children because css_tryget
fails on it but css_offline still not done. This is fixable, though,
and force_empty is just a workaround so I wouldn't see this as a proper
justification to keep it alive.
One possible way to fix this is to iterate children even when css_tryget
fails for them if they haven't finished css_offline yet.
There are some changes in the cgroups core which should make this easier
and Johannes claimed he has some work in that area.
Anyway this is a useful testcase. Thanks Greg!
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID p/c/cgroup.procs exec
sleep 1d )
-- Demonstrate that using memory.force_empty before rmdir, behaves more
sensibly. Stats for reparented child memory are not hidden.
$ /test force
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID p/c/cgroup.procs exec
sleep 1d )
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 19-05-14 16:02:48, Michal Hocko wrote: On Fri 16-05-14 15:00:16, Greg Thelen wrote: [...] -- First, demonstrate that just rmdir, without memory.force_empty, temporarily hides reparented child memory stats. $ /test p/memory.stat:rss 0 p/memory.stat:total_rss 69632 p/c/memory.stat:rss 69632 p/c/memory.stat:total_rss 69632 For a small time the p/c memory has not been reparented to p. p/memory.stat:rss 0 p/memory.stat:total_rss 0 OK, this is a bug. Our iterators skip the children because css_tryget fails on it but css_offline still not done. Or use the cgroups iterator directly. Which would be even easier to fix. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue, May 13 2014, Michal Hocko wrote:
> force_empty has been introduced primarily to drop memory before it gets
> reparented on the group removal. This alone doesn't sound fully
> justified because reparented pages which are not in use can be reclaimed
> also later when there is a memory pressure on the parent level.
>
> Mark the knob CFTYPE_INSANE which tells the cgroup core that it
> shouldn't create the knob with the experimental sane_behavior. Other
> users will get informed about the deprecation and asked to tell us more
> because I do not expect most users will use sane_behavior cgroups mode
> very soon.
> Anyway I expect that most users will be simply cgroup remove handlers
> which do that since ever without having any good reason for it.
>
> If somebody really cares because reparented pages, which would be
> dropped otherwise, push out more important ones then we should fix the
> reparenting code and put pages to the tail.
I should mention a case where I've needed to use memory.force_empty: to
synchronously flush stats from child to parent. Without force_empty
memory.stat is temporarily inconsistent until async css_offline
reparents charges. Here is an example on v3.14 showing that
parent/memory.stat contents are in-flux immediately after rmdir of
parent/child.
$ cat /test
#!/bin/bash
# Create parent and child. Add some non-reclaimable anon rss to child,
# then move running task to parent.
mkdir p p/c
(echo $BASHPID > p/c/cgroup.procs && exec sleep 1d) &
pid=$!
sleep 1
echo $pid > p/cgroup.procs
grep 'rss ' {p,p/c}/memory.stat
if [[ $1 == force ]]; then
echo 1 > p/c/memory.force_empty
fi
rmdir p/c
echo 'For a small time the p/c memory has not been reparented to p.'
grep 'rss ' {p,p/c}/memory.stat
sleep 1
echo 'After waiting all memory has been reparented'
grep 'rss ' {p,p/c}/memory.stat
kill $pid
rmdir p
-- First, demonstrate that just rmdir, without memory.force_empty,
temporarily hides reparented child memory stats.
$ /test
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 0
p/memory.stat:total_rss 0
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID > p/c/cgroup.procs && exec sleep
1d )
-- Demonstrate that using memory.force_empty before rmdir, behaves more
sensibly. Stats for reparented child memory are not hidden.
$ /test force
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID > p/c/cgroup.procs && exec sleep
1d )
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue, May 13 2014, Michal Hocko mho...@suse.cz wrote:
force_empty has been introduced primarily to drop memory before it gets
reparented on the group removal. This alone doesn't sound fully
justified because reparented pages which are not in use can be reclaimed
also later when there is a memory pressure on the parent level.
Mark the knob CFTYPE_INSANE which tells the cgroup core that it
shouldn't create the knob with the experimental sane_behavior. Other
users will get informed about the deprecation and asked to tell us more
because I do not expect most users will use sane_behavior cgroups mode
very soon.
Anyway I expect that most users will be simply cgroup remove handlers
which do that since ever without having any good reason for it.
If somebody really cares because reparented pages, which would be
dropped otherwise, push out more important ones then we should fix the
reparenting code and put pages to the tail.
I should mention a case where I've needed to use memory.force_empty: to
synchronously flush stats from child to parent. Without force_empty
memory.stat is temporarily inconsistent until async css_offline
reparents charges. Here is an example on v3.14 showing that
parent/memory.stat contents are in-flux immediately after rmdir of
parent/child.
$ cat /test
#!/bin/bash
# Create parent and child. Add some non-reclaimable anon rss to child,
# then move running task to parent.
mkdir p p/c
(echo $BASHPID p/c/cgroup.procs exec sleep 1d)
pid=$!
sleep 1
echo $pid p/cgroup.procs
grep 'rss ' {p,p/c}/memory.stat
if [[ $1 == force ]]; then
echo 1 p/c/memory.force_empty
fi
rmdir p/c
echo 'For a small time the p/c memory has not been reparented to p.'
grep 'rss ' {p,p/c}/memory.stat
sleep 1
echo 'After waiting all memory has been reparented'
grep 'rss ' {p,p/c}/memory.stat
kill $pid
rmdir p
-- First, demonstrate that just rmdir, without memory.force_empty,
temporarily hides reparented child memory stats.
$ /test
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 0
p/memory.stat:total_rss 0
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID p/c/cgroup.procs exec sleep
1d )
-- Demonstrate that using memory.force_empty before rmdir, behaves more
sensibly. Stats for reparented child memory are not hidden.
$ /test force
p/memory.stat:rss 0
p/memory.stat:total_rss 69632
p/c/memory.stat:rss 69632
p/c/memory.stat:total_rss 69632
For a small time the p/c memory has not been reparented to p.
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
After waiting all memory has been reparented
p/memory.stat:rss 69632
p/memory.stat:total_rss 69632
grep: p/c/memory.stat: No such file or directory
/test: Terminated ( echo $BASHPID p/c/cgroup.procs exec sleep
1d )
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue 13-05-14 14:39:53, Andrew Morton wrote:
> On Tue, 13 May 2014 17:29:16 +0200 Michal Hocko wrote:
>
> > force_empty has been introduced primarily to drop memory before it gets
> > reparented on the group removal. This alone doesn't sound fully
> > justified because reparented pages which are not in use can be reclaimed
> > also later when there is a memory pressure on the parent level.
> >
> > Mark the knob CFTYPE_INSANE which tells the cgroup core that it
> > shouldn't create the knob with the experimental sane_behavior. Other
> > users will get informed about the deprecation and asked to tell us more
> > because I do not expect most users will use sane_behavior cgroups mode
> > very soon.
> > Anyway I expect that most users will be simply cgroup remove handlers
> > which do that since ever without having any good reason for it.
> >
> > If somebody really cares because reparented pages, which would be
> > dropped otherwise, push out more important ones then we should fix the
> > reparenting code and put pages to the tail.
> >
> > ...
> >
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> > cgroup_subsys_state *css,
> >
> > if (mem_cgroup_is_root(memcg))
> > return -EINVAL;
> > + pr_info("%s (%d): memory.force_empty is deprecated and will be
> > removed.",
> > + current->comm, task_pid_nr(current));
> > + pr_cont(" Let us know if you know if it needed in your usecase at");
> > + pr_cont(" linux...@kvack.org\n");
> > return mem_cgroup_force_empty(memcg);
> > }
> >
>
> Do we really want to spam the poor user each and every time they use
> this? Using pr_info_once() is kinder and gentler?
We do not catch all potential callers but it is true that some
configurations might have thousands of cgroups and the notify_on_release
handler will spam the log.
> From: Andrew Morton
> Subject: memcg-deprecate-memoryforce_empty-knob-fix
>
> - s/pr_info/pr_info_once/
> - fix garbled printk text
>
> Cc: Johannes Weiner
> Cc: Michal Hocko
> Signed-off-by: Andrew Morton
Acked-by: Michal Hocko
> ---
>
> Documentation/cgroups/memory.txt |2 +-
> mm/memcontrol.c |8
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff -puN
> Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix
> Documentation/cgroups/memory.txt
> ---
> a/Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix
> +++ a/Documentation/cgroups/memory.txt
> @@ -482,7 +482,7 @@ About use_hierarchy, see Section 6.
>memory.kmem.usage_in_bytes == memory.usage_in_bytes.
>
>Please note that this knob is considered deprecated and will be removed
> - in future.
> + in the future.
>
>About use_hierarchy, see Section 6.
>
> diff -puN mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix
> mm/memcontrol.c
> --- a/mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix
> +++ a/mm/memcontrol.c
> @@ -4799,10 +4799,10 @@ static int mem_cgroup_force_empty_write(
>
> if (mem_cgroup_is_root(memcg))
> return -EINVAL;
> - pr_info("%s (%d): memory.force_empty is deprecated and will be
> removed.",
> - current->comm, task_pid_nr(current));
> - pr_cont(" Let us know if you know if it needed in your usecase at");
> - pr_cont(" linux...@kvack.org\n");
> + pr_info_once("%s (%d): memory.force_empty is deprecated and will be "
> + "removed. Let us know if it is needed in your usecase at "
> + "linux...@kvack.org\n",
> + current->comm, task_pid_nr(current));
> return mem_cgroup_force_empty(memcg);
> }
>
> _
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue 13-05-14 14:39:53, Andrew Morton wrote: On Tue, 13 May 2014 17:29:16 +0200 Michal Hocko mho...@suse.cz wrote: force_empty has been introduced primarily to drop memory before it gets reparented on the group removal. This alone doesn't sound fully justified because reparented pages which are not in use can be reclaimed also later when there is a memory pressure on the parent level. Mark the knob CFTYPE_INSANE which tells the cgroup core that it shouldn't create the knob with the experimental sane_behavior. Other users will get informed about the deprecation and asked to tell us more because I do not expect most users will use sane_behavior cgroups mode very soon. Anyway I expect that most users will be simply cgroup remove handlers which do that since ever without having any good reason for it. If somebody really cares because reparented pages, which would be dropped otherwise, push out more important ones then we should fix the reparenting code and put pages to the tail. ... --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); } Do we really want to spam the poor user each and every time they use this? Using pr_info_once() is kinder and gentler? We do not catch all potential callers but it is true that some configurations might have thousands of cgroups and the notify_on_release handler will spam the log. From: Andrew Morton a...@linux-foundation.org Subject: memcg-deprecate-memoryforce_empty-knob-fix - s/pr_info/pr_info_once/ - fix garbled printk text Cc: Johannes Weiner han...@cmpxchg.org Cc: Michal Hocko mho...@suse.cz Signed-off-by: Andrew Morton a...@linux-foundation.org Acked-by: Michal Hocko mho...@suse.cz --- Documentation/cgroups/memory.txt |2 +- mm/memcontrol.c |8 2 files changed, 5 insertions(+), 5 deletions(-) diff -puN Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix Documentation/cgroups/memory.txt --- a/Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix +++ a/Documentation/cgroups/memory.txt @@ -482,7 +482,7 @@ About use_hierarchy, see Section 6. memory.kmem.usage_in_bytes == memory.usage_in_bytes. Please note that this knob is considered deprecated and will be removed - in future. + in the future. About use_hierarchy, see Section 6. diff -puN mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix mm/memcontrol.c --- a/mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix +++ a/mm/memcontrol.c @@ -4799,10 +4799,10 @@ static int mem_cgroup_force_empty_write( if (mem_cgroup_is_root(memcg)) return -EINVAL; - pr_info(%s (%d): memory.force_empty is deprecated and will be removed., - current-comm, task_pid_nr(current)); - pr_cont( Let us know if you know if it needed in your usecase at); - pr_cont( linux...@kvack.org\n); + pr_info_once(%s (%d): memory.force_empty is deprecated and will be + removed. Let us know if it is needed in your usecase at + linux...@kvack.org\n, + current-comm, task_pid_nr(current)); return mem_cgroup_force_empty(memcg); } _ -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue, 13 May 2014 17:29:16 +0200 Michal Hocko wrote:
> force_empty has been introduced primarily to drop memory before it gets
> reparented on the group removal. This alone doesn't sound fully
> justified because reparented pages which are not in use can be reclaimed
> also later when there is a memory pressure on the parent level.
>
> Mark the knob CFTYPE_INSANE which tells the cgroup core that it
> shouldn't create the knob with the experimental sane_behavior. Other
> users will get informed about the deprecation and asked to tell us more
> because I do not expect most users will use sane_behavior cgroups mode
> very soon.
> Anyway I expect that most users will be simply cgroup remove handlers
> which do that since ever without having any good reason for it.
>
> If somebody really cares because reparented pages, which would be
> dropped otherwise, push out more important ones then we should fix the
> reparenting code and put pages to the tail.
>
> ...
>
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> cgroup_subsys_state *css,
>
> if (mem_cgroup_is_root(memcg))
> return -EINVAL;
> + pr_info("%s (%d): memory.force_empty is deprecated and will be
> removed.",
> + current->comm, task_pid_nr(current));
> + pr_cont(" Let us know if you know if it needed in your usecase at");
> + pr_cont(" linux...@kvack.org\n");
> return mem_cgroup_force_empty(memcg);
> }
>
Do we really want to spam the poor user each and every time they use
this? Using pr_info_once() is kinder and gentler?
From: Andrew Morton
Subject: memcg-deprecate-memoryforce_empty-knob-fix
- s/pr_info/pr_info_once/
- fix garbled printk text
Cc: Johannes Weiner
Cc: Michal Hocko
Signed-off-by: Andrew Morton
---
Documentation/cgroups/memory.txt |2 +-
mm/memcontrol.c |8
2 files changed, 5 insertions(+), 5 deletions(-)
diff -puN
Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix
Documentation/cgroups/memory.txt
---
a/Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix
+++ a/Documentation/cgroups/memory.txt
@@ -482,7 +482,7 @@ About use_hierarchy, see Section 6.
memory.kmem.usage_in_bytes == memory.usage_in_bytes.
Please note that this knob is considered deprecated and will be removed
- in future.
+ in the future.
About use_hierarchy, see Section 6.
diff -puN mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix
mm/memcontrol.c
--- a/mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix
+++ a/mm/memcontrol.c
@@ -4799,10 +4799,10 @@ static int mem_cgroup_force_empty_write(
if (mem_cgroup_is_root(memcg))
return -EINVAL;
- pr_info("%s (%d): memory.force_empty is deprecated and will be
removed.",
- current->comm, task_pid_nr(current));
- pr_cont(" Let us know if you know if it needed in your usecase at");
- pr_cont(" linux...@kvack.org\n");
+ pr_info_once("%s (%d): memory.force_empty is deprecated and will be "
+"removed. Let us know if it is needed in your usecase at "
+"linux...@kvack.org\n",
+current->comm, task_pid_nr(current));
return mem_cgroup_force_empty(memcg);
}
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue 13-05-14 09:16:56, Johannes Weiner wrote: > On Mon, May 12, 2014 at 05:34:58PM +0200, Michal Hocko wrote: [...] > > >From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001 > > From: Michal Hocko > > Date: Mon, 12 May 2014 16:20:46 +0200 > > Subject: [PATCH] memcg: deprecate memory.force_empty knob > > > > force_empty has been introduced primarily to drop memory before it gets > > reparented on the group removal. This alone doesn't sound fully > > justified because reparented pages which are not in use can be reclaimed > > also later when there is a memory pressure on the parent level. > > > > Mark the knob CFTYPE_INSANE which tells the cgroup core that it > > shouldn't create the knob with the experimental sane_behavior. Other > > users will get informed about the deprecation and asked to tell us more. > > But I expect that most users will be simply cgroup remove handlers > > which do that since ever without having any good reason for it. > > > > If somebody really cares and the reparented pages, which would be dropped > > otherwise, push out more important ones then we should fix the > > reparenting code and put pages to the tail. > > > > Signed-off-by: Michal Hocko > > I'm skeptical the printk will do anything useful, but you marked the > knob insane and that's the most important change. Well, I suspect that most users will try the new semantic at the latest possible moment and then it can come up as a surprise. I would prefer to catch those as soon as possible. I am even thinking to push this to SLES to catch possible enterprise users. > Acked-by: Johannes Weiner Thanks. OK, I will post it to Andrew. I guess he will want to have some rate-limiting or print-once semantic... -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 05:34:58PM +0200, Michal Hocko wrote:
> On Mon 12-05-14 11:25:07, Tejun Heo wrote:
> > On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote:
> > > On Mon 12-05-14 11:00:14, Tejun Heo wrote:
> > > > On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
> > > > > @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> > > > > cgroup_subsys_state *css,
> > > > >
> > > > > if (mem_cgroup_is_root(memcg))
> > > > > return -EINVAL;
> > > > > + pr_info("%s (%d): memory.force_empty is deprecated and will be
> > > > > removed.",
> > > > > + current->comm, task_pid_nr(current));
> > > > > + pr_cont(" Let us know if you know if it needed in your usecase
> > > > > at");
> > > > > + pr_cont(" linux...@kvack.org\n");
> > > > > return mem_cgroup_force_empty(memcg);
> > > >
> > > > It probably would be way easier to just mark the knob with
> > > > CFTYPE_INSANE.
> > >
> > > That would prevent from creating the file, right? I do not mind that but
> > > I would like to see people complaining before.
> >
> > Oh sure, if you wanna see people complaining before the roll out of
> > unified hierarchy, but let's make sure it's also marked with
> > CFTYPE_INSANE. It's easy to remove the flag afterwards. The other
> > way isn't, so...
> ---
> >From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001
> From: Michal Hocko
> Date: Mon, 12 May 2014 16:20:46 +0200
> Subject: [PATCH] memcg: deprecate memory.force_empty knob
>
> force_empty has been introduced primarily to drop memory before it gets
> reparented on the group removal. This alone doesn't sound fully
> justified because reparented pages which are not in use can be reclaimed
> also later when there is a memory pressure on the parent level.
>
> Mark the knob CFTYPE_INSANE which tells the cgroup core that it
> shouldn't create the knob with the experimental sane_behavior. Other
> users will get informed about the deprecation and asked to tell us more.
> But I expect that most users will be simply cgroup remove handlers
> which do that since ever without having any good reason for it.
>
> If somebody really cares and the reparented pages, which would be dropped
> otherwise, push out more important ones then we should fix the
> reparenting code and put pages to the tail.
>
> Signed-off-by: Michal Hocko
I'm skeptical the printk will do anything useful, but you marked the
knob insane and that's the most important change.
Acked-by: Johannes Weiner
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 05:34:58PM +0200, Michal Hocko wrote: On Mon 12-05-14 11:25:07, Tejun Heo wrote: On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote: On Mon 12-05-14 11:00:14, Tejun Heo wrote: On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote: @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); It probably would be way easier to just mark the knob with CFTYPE_INSANE. That would prevent from creating the file, right? I do not mind that but I would like to see people complaining before. Oh sure, if you wanna see people complaining before the roll out of unified hierarchy, but let's make sure it's also marked with CFTYPE_INSANE. It's easy to remove the flag afterwards. The other way isn't, so... --- From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001 From: Michal Hocko mho...@suse.cz Date: Mon, 12 May 2014 16:20:46 +0200 Subject: [PATCH] memcg: deprecate memory.force_empty knob force_empty has been introduced primarily to drop memory before it gets reparented on the group removal. This alone doesn't sound fully justified because reparented pages which are not in use can be reclaimed also later when there is a memory pressure on the parent level. Mark the knob CFTYPE_INSANE which tells the cgroup core that it shouldn't create the knob with the experimental sane_behavior. Other users will get informed about the deprecation and asked to tell us more. But I expect that most users will be simply cgroup remove handlers which do that since ever without having any good reason for it. If somebody really cares and the reparented pages, which would be dropped otherwise, push out more important ones then we should fix the reparenting code and put pages to the tail. Signed-off-by: Michal Hocko mho...@suse.cz I'm skeptical the printk will do anything useful, but you marked the knob insane and that's the most important change. Acked-by: Johannes Weiner han...@cmpxchg.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue 13-05-14 09:16:56, Johannes Weiner wrote: On Mon, May 12, 2014 at 05:34:58PM +0200, Michal Hocko wrote: [...] From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001 From: Michal Hocko mho...@suse.cz Date: Mon, 12 May 2014 16:20:46 +0200 Subject: [PATCH] memcg: deprecate memory.force_empty knob force_empty has been introduced primarily to drop memory before it gets reparented on the group removal. This alone doesn't sound fully justified because reparented pages which are not in use can be reclaimed also later when there is a memory pressure on the parent level. Mark the knob CFTYPE_INSANE which tells the cgroup core that it shouldn't create the knob with the experimental sane_behavior. Other users will get informed about the deprecation and asked to tell us more. But I expect that most users will be simply cgroup remove handlers which do that since ever without having any good reason for it. If somebody really cares and the reparented pages, which would be dropped otherwise, push out more important ones then we should fix the reparenting code and put pages to the tail. Signed-off-by: Michal Hocko mho...@suse.cz I'm skeptical the printk will do anything useful, but you marked the knob insane and that's the most important change. Well, I suspect that most users will try the new semantic at the latest possible moment and then it can come up as a surprise. I would prefer to catch those as soon as possible. I am even thinking to push this to SLES to catch possible enterprise users. Acked-by: Johannes Weiner han...@cmpxchg.org Thanks. OK, I will post it to Andrew. I guess he will want to have some rate-limiting or print-once semantic... -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Tue, 13 May 2014 17:29:16 +0200 Michal Hocko mho...@suse.cz wrote: force_empty has been introduced primarily to drop memory before it gets reparented on the group removal. This alone doesn't sound fully justified because reparented pages which are not in use can be reclaimed also later when there is a memory pressure on the parent level. Mark the knob CFTYPE_INSANE which tells the cgroup core that it shouldn't create the knob with the experimental sane_behavior. Other users will get informed about the deprecation and asked to tell us more because I do not expect most users will use sane_behavior cgroups mode very soon. Anyway I expect that most users will be simply cgroup remove handlers which do that since ever without having any good reason for it. If somebody really cares because reparented pages, which would be dropped otherwise, push out more important ones then we should fix the reparenting code and put pages to the tail. ... --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); } Do we really want to spam the poor user each and every time they use this? Using pr_info_once() is kinder and gentler? From: Andrew Morton a...@linux-foundation.org Subject: memcg-deprecate-memoryforce_empty-knob-fix - s/pr_info/pr_info_once/ - fix garbled printk text Cc: Johannes Weiner han...@cmpxchg.org Cc: Michal Hocko mho...@suse.cz Signed-off-by: Andrew Morton a...@linux-foundation.org --- Documentation/cgroups/memory.txt |2 +- mm/memcontrol.c |8 2 files changed, 5 insertions(+), 5 deletions(-) diff -puN Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix Documentation/cgroups/memory.txt --- a/Documentation/cgroups/memory.txt~memcg-deprecate-memoryforce_empty-knob-fix +++ a/Documentation/cgroups/memory.txt @@ -482,7 +482,7 @@ About use_hierarchy, see Section 6. memory.kmem.usage_in_bytes == memory.usage_in_bytes. Please note that this knob is considered deprecated and will be removed - in future. + in the future. About use_hierarchy, see Section 6. diff -puN mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix mm/memcontrol.c --- a/mm/memcontrol.c~memcg-deprecate-memoryforce_empty-knob-fix +++ a/mm/memcontrol.c @@ -4799,10 +4799,10 @@ static int mem_cgroup_force_empty_write( if (mem_cgroup_is_root(memcg)) return -EINVAL; - pr_info(%s (%d): memory.force_empty is deprecated and will be removed., - current-comm, task_pid_nr(current)); - pr_cont( Let us know if you know if it needed in your usecase at); - pr_cont( linux...@kvack.org\n); + pr_info_once(%s (%d): memory.force_empty is deprecated and will be +removed. Let us know if it is needed in your usecase at +linux...@kvack.org\n, +current-comm, task_pid_nr(current)); return mem_cgroup_force_empty(memcg); } _ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 12-05-14 11:25:07, Tejun Heo wrote:
> On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote:
> > On Mon 12-05-14 11:00:14, Tejun Heo wrote:
> > > On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
> > > > @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> > > > cgroup_subsys_state *css,
> > > >
> > > > if (mem_cgroup_is_root(memcg))
> > > > return -EINVAL;
> > > > + pr_info("%s (%d): memory.force_empty is deprecated and will be
> > > > removed.",
> > > > + current->comm, task_pid_nr(current));
> > > > + pr_cont(" Let us know if you know if it needed in your usecase
> > > > at");
> > > > + pr_cont(" linux...@kvack.org\n");
> > > > return mem_cgroup_force_empty(memcg);
> > >
> > > It probably would be way easier to just mark the knob with
> > > CFTYPE_INSANE.
> >
> > That would prevent from creating the file, right? I do not mind that but
> > I would like to see people complaining before.
>
> Oh sure, if you wanna see people complaining before the roll out of
> unified hierarchy, but let's make sure it's also marked with
> CFTYPE_INSANE. It's easy to remove the flag afterwards. The other
> way isn't, so...
---
>From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001
From: Michal Hocko
Date: Mon, 12 May 2014 16:20:46 +0200
Subject: [PATCH] memcg: deprecate memory.force_empty knob
force_empty has been introduced primarily to drop memory before it gets
reparented on the group removal. This alone doesn't sound fully
justified because reparented pages which are not in use can be reclaimed
also later when there is a memory pressure on the parent level.
Mark the knob CFTYPE_INSANE which tells the cgroup core that it
shouldn't create the knob with the experimental sane_behavior. Other
users will get informed about the deprecation and asked to tell us more.
But I expect that most users will be simply cgroup remove handlers
which do that since ever without having any good reason for it.
If somebody really cares and the reparented pages, which would be dropped
otherwise, push out more important ones then we should fix the
reparenting code and put pages to the tail.
Signed-off-by: Michal Hocko
---
Documentation/cgroups/memory.txt | 3 +++
mm/memcontrol.c | 5 +
2 files changed, 8 insertions(+)
diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index f0f67b44ea07..fc9fad984bfb 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -477,6 +477,9 @@ About use_hierarchy, see Section 6.
write will still return success. In this case, it is expected that
memory.kmem.usage_in_bytes == memory.usage_in_bytes.
+ Please note that this knob is considered deprecated and will be removed
+ in future.
+
About use_hierarchy, see Section 6.
5.2 stat file
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b030b15b626a..ee123f3d40d5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
cgroup_subsys_state *css,
if (mem_cgroup_is_root(memcg))
return -EINVAL;
+ pr_info("%s (%d): memory.force_empty is deprecated and will be
removed.",
+ current->comm, task_pid_nr(current));
+ pr_cont(" Let us know if you know if it needed in your usecase at");
+ pr_cont(" linux...@kvack.org\n");
return mem_cgroup_force_empty(memcg);
}
@@ -6037,6 +6041,7 @@ static struct cftype mem_cgroup_files[] = {
},
{
.name = "force_empty",
+ .flags = CFTYPE_INSANE,
.trigger = mem_cgroup_force_empty_write,
},
{
--
2.0.0.rc0
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote:
> On Mon 12-05-14 11:00:14, Tejun Heo wrote:
> > On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
> > > @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> > > cgroup_subsys_state *css,
> > >
> > > if (mem_cgroup_is_root(memcg))
> > > return -EINVAL;
> > > + pr_info("%s (%d): memory.force_empty is deprecated and will be
> > > removed.",
> > > + current->comm, task_pid_nr(current));
> > > + pr_cont(" Let us know if you know if it needed in your usecase at");
> > > + pr_cont(" linux...@kvack.org\n");
> > > return mem_cgroup_force_empty(memcg);
> >
> > It probably would be way easier to just mark the knob with
> > CFTYPE_INSANE.
>
> That would prevent from creating the file, right? I do not mind that but
> I would like to see people complaining before.
Oh sure, if you wanna see people complaining before the roll out of
unified hierarchy, but let's make sure it's also marked with
CFTYPE_INSANE. It's easy to remove the flag afterwards. The other
way isn't, so...
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 12-05-14 11:00:14, Tejun Heo wrote:
> On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
> > @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> > cgroup_subsys_state *css,
> >
> > if (mem_cgroup_is_root(memcg))
> > return -EINVAL;
> > + pr_info("%s (%d): memory.force_empty is deprecated and will be
> > removed.",
> > + current->comm, task_pid_nr(current));
> > + pr_cont(" Let us know if you know if it needed in your usecase at");
> > + pr_cont(" linux...@kvack.org\n");
> > return mem_cgroup_force_empty(memcg);
>
> It probably would be way easier to just mark the knob with
> CFTYPE_INSANE.
That would prevent from creating the file, right? I do not mind that but
I would like to see people complaining before.
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
> @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
> cgroup_subsys_state *css,
>
> if (mem_cgroup_is_root(memcg))
> return -EINVAL;
> + pr_info("%s (%d): memory.force_empty is deprecated and will be
> removed.",
> + current->comm, task_pid_nr(current));
> + pr_cont(" Let us know if you know if it needed in your usecase at");
> + pr_cont(" linux...@kvack.org\n");
> return mem_cgroup_force_empty(memcg);
It probably would be way easier to just mark the knob with
CFTYPE_INSANE.
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote: @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); It probably would be way easier to just mark the knob with CFTYPE_INSANE. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 12-05-14 11:00:14, Tejun Heo wrote: On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote: @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); It probably would be way easier to just mark the knob with CFTYPE_INSANE. That would prevent from creating the file, right? I do not mind that but I would like to see people complaining before. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote: On Mon 12-05-14 11:00:14, Tejun Heo wrote: On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote: @@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct cgroup_subsys_state *css, if (mem_cgroup_is_root(memcg)) return -EINVAL; + pr_info(%s (%d): memory.force_empty is deprecated and will be removed., + current-comm, task_pid_nr(current)); + pr_cont( Let us know if you know if it needed in your usecase at); + pr_cont( linux...@kvack.org\n); return mem_cgroup_force_empty(memcg); It probably would be way easier to just mark the knob with CFTYPE_INSANE. That would prevent from creating the file, right? I do not mind that but I would like to see people complaining before. Oh sure, if you wanna see people complaining before the roll out of unified hierarchy, but let's make sure it's also marked with CFTYPE_INSANE. It's easy to remove the flag afterwards. The other way isn't, so... Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] memcg: deprecate memory.force_empty knob
On Mon 12-05-14 11:25:07, Tejun Heo wrote:
On Mon, May 12, 2014 at 05:20:15PM +0200, Michal Hocko wrote:
On Mon 12-05-14 11:00:14, Tejun Heo wrote:
On Mon, May 12, 2014 at 04:58:03PM +0200, Michal Hocko wrote:
@@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
cgroup_subsys_state *css,
if (mem_cgroup_is_root(memcg))
return -EINVAL;
+ pr_info(%s (%d): memory.force_empty is deprecated and will be
removed.,
+ current-comm, task_pid_nr(current));
+ pr_cont( Let us know if you know if it needed in your usecase
at);
+ pr_cont( linux...@kvack.org\n);
return mem_cgroup_force_empty(memcg);
It probably would be way easier to just mark the knob with
CFTYPE_INSANE.
That would prevent from creating the file, right? I do not mind that but
I would like to see people complaining before.
Oh sure, if you wanna see people complaining before the roll out of
unified hierarchy, but let's make sure it's also marked with
CFTYPE_INSANE. It's easy to remove the flag afterwards. The other
way isn't, so...
---
From 6f2a33df7750f0794b03f7a85aba02a4e631f2a0 Mon Sep 17 00:00:00 2001
From: Michal Hocko mho...@suse.cz
Date: Mon, 12 May 2014 16:20:46 +0200
Subject: [PATCH] memcg: deprecate memory.force_empty knob
force_empty has been introduced primarily to drop memory before it gets
reparented on the group removal. This alone doesn't sound fully
justified because reparented pages which are not in use can be reclaimed
also later when there is a memory pressure on the parent level.
Mark the knob CFTYPE_INSANE which tells the cgroup core that it
shouldn't create the knob with the experimental sane_behavior. Other
users will get informed about the deprecation and asked to tell us more.
But I expect that most users will be simply cgroup remove handlers
which do that since ever without having any good reason for it.
If somebody really cares and the reparented pages, which would be dropped
otherwise, push out more important ones then we should fix the
reparenting code and put pages to the tail.
Signed-off-by: Michal Hocko mho...@suse.cz
---
Documentation/cgroups/memory.txt | 3 +++
mm/memcontrol.c | 5 +
2 files changed, 8 insertions(+)
diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index f0f67b44ea07..fc9fad984bfb 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -477,6 +477,9 @@ About use_hierarchy, see Section 6.
write will still return success. In this case, it is expected that
memory.kmem.usage_in_bytes == memory.usage_in_bytes.
+ Please note that this knob is considered deprecated and will be removed
+ in future.
+
About use_hierarchy, see Section 6.
5.2 stat file
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b030b15b626a..ee123f3d40d5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4793,6 +4793,10 @@ static int mem_cgroup_force_empty_write(struct
cgroup_subsys_state *css,
if (mem_cgroup_is_root(memcg))
return -EINVAL;
+ pr_info(%s (%d): memory.force_empty is deprecated and will be
removed.,
+ current-comm, task_pid_nr(current));
+ pr_cont( Let us know if you know if it needed in your usecase at);
+ pr_cont( linux...@kvack.org\n);
return mem_cgroup_force_empty(memcg);
}
@@ -6037,6 +6041,7 @@ static struct cftype mem_cgroup_files[] = {
},
{
.name = force_empty,
+ .flags = CFTYPE_INSANE,
.trigger = mem_cgroup_force_empty_write,
},
{
--
2.0.0.rc0
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
