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 >> >> . >> >
------------------------------------------------------------------------------ Time is money. Stop wasting it! Get your web API in 5 minutes. www.restlet.com/download http://p.sf.net/sfu/restlet _______________________________________________ Libcg-devel mailing list Libcg-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libcg-devel
