On 01/15/2013 02:25 PM, Serge Hallyn wrote:
> Quoting Jeff Liu (jeff.liu@xxxxxxxxxx):
>> Hello,
>>
>> Currently, the pid list shown on cgroup->procs & tasks is in ascending order. However, this list is not
>> guaranteed to be sorted according to the following records mentioned at Documentation/cgroups/cgroups.txt.
>>
>> - tasks: list of tasks (by PID) attached to that cgroup. This list
>> is not guaranteed to be sorted.
>> - cgroup.procs: list of thread group IDs in the cgroup. This list is
>> not guaranteed to be sorted or free of duplicate TGIDs, and userspace
>> should sort/uniquify the list if this property is required.
>>
>> This patch remove the sorting function to make the default behavior of pid list be consistent with
>> the document.
>>
>> Without patch:
>> # echo 2942 >> /cgroup/test/tasks
>> # echo 2803 >> /cgroup/test/tasks
>> # echo 2911 >> /cgroup/test/tasks
>> # echo 2761 >> /cgroup/test/tasks
>> # cat /cgroup/test/tasks
>> 2761
>> 2803
>> 2911
>> 2942
>> # cat /cgroup/test/cgroup.procs
>> 2761
>> 2803
>> 2911
>> 2942
>>
>> Patch applied:
>> # echo 3042 >> /cgroup/testfixed/tasks
>> # echo 2916 >> /cgroup/testfixed/tasks
>> # echo 2781 >> /cgroup/testfixed/tasks
>> # echo 2527 >> /cgroup/testfixed/tasks
>> # cat /cgroup/testfixed/tasks
>> 2916
>> 3042
>> 2527
>> 2781
>> # cat /cgroup/testfixed/cgroup.procs
>> 2916
>> 3042
>> 2527
>> 2781
>>
>>
>> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>>
>> ---
>> kernel/cgroup.c | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
>> index f24f724..2b5b477 100644
>> --- a/kernel/cgroup.c
>> +++ b/kernel/cgroup.c
>> @@ -47,7 +47,6 @@
>> #include <linux/magic.h>
>> #include <linux/spinlock.h>
>> #include <linux/string.h>
>> -#include <linux/sort.h>
>> #include <linux/kmod.h>
>> #include <linux/module.h>
>> #include <linux/delayacct.h>
>> @@ -3374,11 +3373,6 @@ after:
>> return dest;
>> }
>>
>> -static int cmppid(const void *a, const void *b)
>> -{
>> - return *(pid_t *)a - *(pid_t *)b;
>> -}
>> -
>> /*
>> * find the appropriate pidlist for our purpose (given procs vs tasks)
>> * returns with the lock on that pidlist already held, and takes care
>> @@ -3463,8 +3457,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
>> }
>> cgroup_iter_end(cgrp, &it);
>> length = n;
>> - /* now sort & (if procs) strip out duplicates */
>> - sort(array, length, sizeof(pid_t), cmppid, NULL);
>> + /* if procs, strip out duplicates */
>> if (type == CGROUP_FILE_PROCS)
>> length = pidlist_uniq(&array, length);
>
> But... pidlist_uniq() depends on array being sorted
Yes, I forgot removing this function according to cgroup.procs documents.
cgroup.procs: list of thread group IDs in the cgroup. This list is
not guaranteed to be sorted or free of duplicate TGIDs
We should remove pidlist_uniq() as well.
Thanks,
-Jeff
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
