On Wed, 20 Aug 2008 13:20:40 +0800
Li Zefan <[EMAIL PROTECTED]> wrote:
> Balbir Singh wote:
> > * Jiri Slaby <[EMAIL PROTECTED]> [2008-08-14 22:16:53]:
> >
> >> Hi,
> >>
> >> found this in mmotm, a fix for
> >> mm-owner-fix-race-between-swap-and-exit.patch
> >>
> >
> > Does the patch below fix your problem, it's against mmotm 19th August
> > 2008.
> >
>
> I just triggered this bug. I think you also need the following change:
>
> make memrlimit_cgroup_mm_owner_changed() aware that old_cgrp can be NULL,
> and note we can't call memrlimit_cgroup_from_cgrp() with NULL argument.
>
> Signed-off-by: Li Zefan <[EMAIL PROTECTED]>
>
Thank you, it seems you fixed a problem which I met in my test.
(*) When I migrate task under cpuset repeatedly, I can see Oops easily.
Tested-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]>
> diff --git a/mm/memrlimitcgroup.c.orig b/mm/memrlimitcgroup.c
> index f7536dc..6559470 100644
> --- a/mm/memrlimitcgroup.c.orig
> +++ b/mm/memrlimitcgroup.c
> @@ -249,17 +249,23 @@ static void memrlimit_cgroup_mm_owner_changed(struct
> cgroup_subsys *ss,
> struct mm_struct *mm = get_task_mm(p);
>
> BUG_ON(!mm);
> - memrcg = memrlimit_cgroup_from_cgrp(cgrp);
> - old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);
>
> /*
> * If we don't have a new cgroup, we just uncharge from the old one.
> * It means that the task is going away
> */
> - if (memrcg &&
> - res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
> - goto out;
> - res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
> + if (cgrp) {
> + memrcg = memrlimit_cgroup_from_cgrp(cgrp);
> + if (res_counter_charge(&memrcg->as_res,
> + mm->total_vm << PAGE_SHIFT))
> + goto out;
> + }
> +
> + if (old_cgrp) {
> + old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);
> + res_counter_uncharge(&old_memrcg->as_res,
> + mm->total_vm <<PAGE_SHIFT);
> + }
> out:
> mmput(mm);
> }
>
>
> >
> > Reported-by: [EMAIL PROTECTED]
> >
> > Jiri reported a problem and saw an oops when the
> > memrlimit-fix-race-with-swap
> > patch is applied. He sent his patch on top to fix the problem, but ran into
> > another issue. The root cause of the problem is that we are not suppose
> > to call task_cgroup on NULL tasks. This patch reverts Jiri's patch and
> > does not call task_cgroup if the passed task_struct (old) is NULL.
> >
> >
> > Signed-off-by: Balbir Singh <[EMAIL PROTECTED]>
> > ---
> >
> > kernel/cgroup.c | 5 +++--
> > kernel/exit.c | 2 +-
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff -puN kernel/exit.c~memrlimit-fix-race-with-swap-oops kernel/exit.c
> > --- linux-2.6.27-rc3/kernel/exit.c~memrlimit-fix-race-with-swap-oops
> > 2008-08-19 18:50:39.000000000 +0530
> > +++ linux-2.6.27-rc3-balbir/kernel/exit.c 2008-08-19 18:51:05.000000000
> > +0530
> > @@ -641,8 +641,8 @@ retry:
> > * the callback and take action
> > */
> > down_write(&mm->mmap_sem);
> > - cgroup_mm_owner_callbacks(mm->owner, NULL);
> > mm->owner = NULL;
> > + cgroup_mm_owner_callbacks(mm->owner, NULL);
> > up_write(&mm->mmap_sem);
> > return;
> >
> > diff -puN kernel/cgroup.c~memrlimit-fix-race-with-swap-oops kernel/cgroup.c
> > --- linux-2.6.27-rc3/kernel/cgroup.c~memrlimit-fix-race-with-swap-oops
> > 2008-08-19 18:50:39.000000000 +0530
> > +++ linux-2.6.27-rc3-balbir/kernel/cgroup.c 2008-08-19 18:55:38.000000000
> > +0530
> > @@ -2743,13 +2743,14 @@ void cgroup_fork_callbacks(struct task_s
> > */
> > void cgroup_mm_owner_callbacks(struct task_struct *old, struct task_struct
> > *new)
> > {
> > - struct cgroup *oldcgrp, *newcgrp = NULL;
> > + struct cgroup *oldcgrp = NULL, *newcgrp = NULL;
> >
> > if (need_mm_owner_callback) {
> > int i;
> > for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
> > struct cgroup_subsys *ss = subsys[i];
> > - oldcgrp = task_cgroup(old, ss->subsys_id);
> > + if (old)
> > + oldcgrp = task_cgroup(old, ss->subsys_id);
> > if (new)
> > newcgrp = task_cgroup(new, ss->subsys_id);
> > if (oldcgrp == newcgrp)
> > diff -puN mm/memrlimitcgroup.c~memrlimit-fix-race-with-swap-oops
> > mm/memrlimitcgroup.c
> > _
> >
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [EMAIL PROTECTED] For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[EMAIL PROTECTED]"> [EMAIL PROTECTED] </a>
>
_______________________________________________
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
