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 > > . > ------------------------------------------------------------------------------ "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
