Hi Balbir,

Thank you for the comment.

Balbir Singh wrote:
>> Changelog of v2:
>>  * Change only [PATCH 2/2] and there is not any changes in [PATCH 1/2]. 
>>  * Use clock_gettime(2) for getting timestamp since a system boot.
>>  * Change parent_info's memory to dynamic allocation.
>>
>> Hi,
>>
>> I have been testing a cgrulesengd daemon and I noticed it fails to
>> change the cgroup of child occasionally. I tested it by following
>> configulation file:
>>
>> /etc/cgrules.conf:
>>     user01          cpuset          group01/user01
>>     %               memory          group01/user01
>>
>> A cpuset subsystem and a memory subsystem are mounted on different
>> mount points, and a cgrulesengd daemon manages each subsystem.
>> I login this environment as a user "user01", and each susbystem's
>> tasks file is the following:
>>
>>  # cat /mnt/cgroups/cpuset/group01/user01/tasks
>>  31801
>>  31805
>>  31806
>>  #
>>  # cat /mnt/cgroups/memory/group01/user01/tasks
>>  31801
>>  31805
>>  #
>>  # pstree -p 32105
>>  sshd(31801)---sshd(31805)---bash(31806)
>>  #
>>
>> They should be the same, but they are different. I investigated this
>> problem, and I found the cause. The reason is that the process(31806)
>> was forked just after writing the process(31805) to a cpuset subsystem's
>> tasks file:
>>
>>  <1> The UID/GID CHANGE event of the process 31805 happens.
>>  <2> The daemon writes "31805" to a cpuset subsystem's tasks file.
>>  <3> The process 31806 is forked, and it appears on a cpuset subsystem's
>>      tasks file.
>>  <4> The daemon writes "31805" to a memory subsystem's tasks file.
>>  <5> The process 31806 does not appears on a memory subsystem's tasks file.
>>
>>
>> For solving this problem, I propose the following sequence.
>>  1. Store both the timestamp and the process-id when the step <4>.
>>  2. If receiving a PROC_EVENT_FORK packet, check its parent-pid and its
>>     timestamp.
>>  3. If its parent-pid and the stored process-id are same and its timestamp
>>     is older than the stored timestamp, change the cgroup of forked process.
>>
> 
> Hi, Ken'ichi
> 
> The overall approach sounds right, however I am wondering why we don't
> use the kernel timestamp and unique sequence number returned by proc
> events. The sequence number is per CPU, but the timestamp present
> should help us
> 
> snippet of proc_event code for UID event notification
> 
>         ktime_get_ts(&ts); /* get high res monotonic timestamp */
>         put_unaligned(timespec_to_ns(&ts), (__u64*)&ev->timestamp_ns);
> 
> Why do we need clock_gettime()?

We need clock_gettime() for getting a timestamp when cgrulesengd daemon
finished changing cgroup, because the daemon should notice a fork event
happening while it changing cgroup.

For example, I describe a UID event and a Fork event of this race problem
in the following sequence table:

 In the following table,
   * TS means timestamp.
   * Pro-A means some process named A.
   * Pro-B means a process B which is a child of process A.

 TS |         KERNEL            |        CGRULESENGD DAEMON
----+---------------------------+----------------------------------------
 10 | Pro-A's UID event happen. | 
 20 |                           | Receive a UID event packet. (TS is 10)
 30 |                           | Change Pro-A's cgroup of cpuset.
 40 | Pro-A forks a new Pro-B.  |
 50 |                           | Change Pro-A's cgroup of memory.
 60 |                           | [Store the pid of Pro-A and TS 60 by my patch]
 70 |                           | Receive a FORK event packet. (TS is 40)

On my patch,
1. (TS:60) A cgrulesengd daemon stores the pid of process A and the
   timestamp (60). *This timestamp is taken by clock_gettime().*
2. (TS:70) The daemon receives a FORK event packet, the daemon checks
   the parent pid and the timestamp (40). The daemon can know that a
   FORK event happened while the daemon changes the cgroup, because the
   timestamp (40) is older/smaller than the stored timestamp (60).
   So the daemon can change it into the correct cgroup.

Does this answer your question ?


> Second question, when we queue all the timestamps, when do we free them?
> Is the race window very small?

We free old queued timestamps when recieving a new fork event in
cgre_remove_old_parent_info().


Thanks
Ken'ichi Ohmichi

------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image 
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to