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 ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Libcg-devel mailing list Libcg-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libcg-devel
