Rearin this issue I agree with Jan. Ivana ----- Original Message ----- > From: "Weng Meiling" <wengmeiling.w...@huawei.com> > To: "Libcg Development list" <libcg-devel@lists.sourceforge.net> > Cc: "Dhaval Giani" <dhaval.gi...@gmail.com>, "Ivana Hutarova Varekova" > <varek...@redhat.com>, "libo chen" > <libo.c...@huawei.com>, "Li Zefan" <lize...@huawei.com>, "yangjieyj yang" > <yangjieyj.y...@huawei.com>, "Jan > Safranek" <jsafr...@redhat.com>, "Balbir Singh" <bsinghar...@gmail.com> > Sent: Friday, June 27, 2014 10:58:12 AM > Subject: [RFC]cgred: fix cgred migrate forked process > > ping... > On 2014/5/19 10:16, Weng Meiling wrote: > > Hi Jan, Ivana, Vivek, Balbir, > > > > What do you think about the problem? > > > > On 2014/5/2 3:26, Dhaval Giani wrote: > >> On Mon, Apr 28, 2014 at 10:05 PM, Weng Meiling > >> <wengmeiling.w...@huawei.com> wrote: > >>> On 2014/4/29 3:05, Dhaval Giani wrote: > >>>> Hi Weng, > >>>> > >>>> I apologize for our tardiness is responding back! > >>>> > >>>> On Sun, Apr 27, 2014 at 9:09 PM, Weng Meiling > >>>> <wengmeiling.w...@huawei.com> wrote: > >>>>> Hi Ivana, > >>>>> What do you think about this problem? > >>>>> > >>>>> Weng Meiling > >>>>> Thanks! > >>>>> > >>>>> On 2014/4/22 11:06, Weng Meiling wrote: > >>>>>> ping... > >>>>>> On 2014/4/10 16:09, Weng Meiling wrote: > >>>>>>> Cgred just migrate forked process when the parent's cgroup is > >>>>>>> changed, > >>>>>>> so that the forked process can stay in the same cgroup which match > >>>>>>> the > >>>>>>> cgred rules with it's parent. But there is a situation when parent > >>>>>>> process > >>>>>>> fork a child process, and exited before it moved into the cgroup > >>>>>>> matched > >>>>>>> the rules successfullly. The forked child process is also failed into > >>>>>>> the > >>>>>>> cgroup. This result is inconsistent with rules. So this patch fix the > >>>>>>> problem. Just apply rule for forked process when parent process > >>>>>>> exited > >>>>>>> when forking. > >>>>>>> > >>>> > >>>> I am not sure of the exact race here. > >>>> > >>>> Parent Forks & Exits > >>>> Child is not classified? > >>>> > >>>> Can you clarify the exact race? > >>>> > >>> The exact race should be parent exit & move to group. > >>> The race may cause child is not classified as rules. > >>> > >>> I can used a example to clarify it: > >>> > >>> start cgred service and the rule: > >>> > >>> root:testd memory test1 > >>> > >>> We have a testd service, and after it starts, it will fork a child > >>> process > >>> and exited immediately. The right result should be the testd process (the > >>> parent > >>> and the child) is in memory:test1 according to the rule. But the real > >>> result is > >>> sometime it is not. > >>> > >>> The race is as following: > >>> > >>> parent: cgred: > >>> > >>> fork() > >>> exit() > >>> move process ------ the parent process exited > >>> before it has been moved to the group. > >>> > >> > >> Sorry for misunderstanding the race. > >> > >>> child move based on event PROC_EVENT_FORK, because the parent has exited, > >>> so > >>> cgre_was_parent_changed_when_forking() can't get it's parent's info and > >>> return > >>> 0, this will result in the rule will not be executed. So the child > >>> process will > >>> stay the same group with it's parent, But the parent was not moved to the > >>> specified > >>> group before exited, so the child is not in the specified group too. From > >>> the user's > >>> point of view, the result doesn't match the rule. > >>> > >>> The patch add check if parents exits, if parent has exited in > >>> cgre_was_parent_changed_when_forking(), > >>> then returns 1 so that the rule will be executed and the child process > >>> can be moved > >>> to the specified group. > >>> > >> > >> So this idea sounds really racy to me. The only sure way it can be > >> ensured is *if* the child runs before the parent exits. I know there > >> is a kernel tunable for that, but I am not sure if it is a good idea. > >> But just for fun, could you try > >> (as root) > >> echo 1 > /proc/sys/kernel/sched_child_runs_first > >> > >> (I am just fishing here), but if you are unable to reproduce the issue > >> here, we at least have a workaround for now. > >> > >>> > >>> > >>> > >>>>>>> But I'm not sure whether the fix is suitable. Because the other > >>>>>>> way(just remove > >>>>>>> the cgre_was_parent_changed_when_forking() for forked processes) will > >>>>>>> affect > >>>>>>> cgclassify --sticky. But this way is not which I have done some test. > >>>>>>> > >>>>>>> Signed-off-by: Weng Meiling <wengmeiling.w...@huawei.com> > >>>>>>> --- > >>>>>>> src/daemon/cgrulesengd.c | 5 +++++ > >>>>>>> 1 file changed, 5 insertions(+) > >>>>>>> > >>>>>>> diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c > >>>>>>> index 367b898..5e254c8 100644 > >>>>>>> --- a/src/daemon/cgrulesengd.c > >>>>>>> +++ b/src/daemon/cgrulesengd.c > >>>>>>> @@ -265,6 +265,8 @@ static int > >>>>>>> cgre_was_parent_changed_when_forking(const struct proc_event *ev) > >>>>>>> pid_t parent_pid; > >>>>>>> __u64 timestamp_child; > >>>>>>> __u64 timestamp_parent; > >>>>>>> + char path[FILENAME_MAX]; > >>>>>>> + struct stat buff_stat; > >>>>>>> > >>>>>>> parent_pid = ev->event_data.fork.parent_pid; > >>>>>>> timestamp_child = ev->timestamp_ns; > >>>>>>> @@ -279,6 +281,9 @@ static int > >>>>>>> cgre_was_parent_changed_when_forking(const struct proc_event *ev) > >>>>>>> continue; > >>>>>>> return 1; > >>>>>>> } > >>>>>>> + sprintf(path, "/proc/%d", parent_pid); > >>>> > >>>> I would prefer to use one of the "n" versions (so snprintf here) > >>>> > >>>>>>> + if (stat(path, &buff_stat) < 0) > >>>>>>> + return 1; > >>>> > >>>> As far as I can see, all you are checking here is if the parent > >>>> exists. What is the the parent dies immediately after this check? > >>>> > >>> > >>> Oh, if parent dies immediately after this check, the problem is still > >>> exists. > >>> So the fix way is not suitable. Do you have any idea? > >>> > >> > >> Unfortunately not. Jan, Ivana, Vivek, Balbir? > >> > >> Thanks! > >> Dhaval > >> > >> . > >> > > > > >
------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft _______________________________________________ Libcg-devel mailing list Libcg-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libcg-devel
