cgroup: don't show pid list on tasks/procs in ascending order
PATCH 1/1 cgroup: don't show pid list on tasks/procs in ascending order
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
--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
kernel/cgroup.c +1 -8
@@ -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); l =cgroup_pidlist_find (cgrp, type);
Signature · 1 line
1.7.9.5
A message that is not in the archive (sent off-list or not yet received): [email protected]
Sorry, I forgot removing the codes regarding free of duplicate TGIDs at cgroup.procs, will re-sent another patch later.
Thanks,
-Jeff
Quoted 88 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +1
-8@@ -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); l =cgroup_pidlist_find (cgrp, type);
Quoted 78 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +0
-6@@ -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
kernel/cgroup.c static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type, · line 3462
Quoted 8 lines
kernel/cgroup.c +1
-2@@ -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
-serge
Quoted 81 lines · Serge Hallyn
Quoted 80 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +0
-6@@ -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);
Quoted 8 lines
kernel/cgroup.c static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type, · line 3462
Quoted 6 lines
kernel/cgroup.c +1
-2length = 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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 7 lines · Jeff Liu
Quoted 6 lines · Serge Hallyn
Quoted 5 lines · Jeff Liu
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 atDocumentation/cgroups/cgroups.txt.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
- 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.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
-serge
Quoted 14 lines · Serge Hallyn
Quoted 13 lines · Jeff Liu
Quoted 12 lines · Serge Hallyn
Quoted 11 lines · Jeff Liu
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 atDocumentation/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.
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
This patch remove the sorting function to make the default behavior of pid list be consistent with the document.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
So that we can clean out around 70-lines source code as well as shrink the cgroup.o a bit.
Thanks,
-Jeff
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 26 lines · Jeff Liu
Quoted 14 lines · Serge Hallyn
Quoted 13 lines · Jeff Liu
Quoted 12 lines · Serge Hallyn
Quoted 11 lines · Jeff Liu
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 atDocumentation/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.Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
This patch remove the sorting function to make the default behavior of pid list be consistent with the document.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
Quoted 6 lines
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
-serge
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Thanks.
Signature · 1 line
tejun
A message that is not in the archive (sent off-list or not yet received): 20130115154332.GA2598-Gd/HAXX7CRxy/[email protected]
Quoted 19 lines · Tejun Heo
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
Quoted 3 lines
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Phew :)
Seriously, once we switch over to using a library anyway, it makes sense. Now I do see that removing the sorting would help move people over to using a library... is there a separate place where the library is being discussed/designed? Requirements being gathered?
-serge
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hey, Serge.
Quoted 4 lines · Serge E. Hallyn
Seriously, once we switch over to using a library anyway, it makes sense. Now I do see that removing the sorting would help move people over to using a library... is there a separate place where the library is being discussed/designed? Requirements being gathered?
It's still just a pipe dream of mine. I have some requirements on mind but don't yet have any concrete plan on how to actually do it. For me, it's queued after unified hierarchy support in kernel, which I think we're progressing towards better than I originally expected. If anyone is intereted in jumping in, please be my guest.
Thanks.
Signature · 1 line
tejun
Hey, Serge.
Quoted 4 lines · Serge E. Hallyn
Seriously, once we switch over to using a library anyway, it makes sense. Now I do see that removing the sorting would help move people over to using a library... is there a separate place where the library is being discussed/designed? Requirements being gathered?
It's still just a pipe dream of mine. I have some requirements on mind but don't yet have any concrete plan on how to actually do it. For me, it's queued after unified hierarchy support in kernel, which I think we're progressing towards better than I originally expected. If anyone is intereted in jumping in, please be my guest.
Thanks.
Signature · 1 line
tejun
Quoted 19 lines · Tejun Heo
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
Quoted 3 lines
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Phew :)
Seriously, once we switch over to using a library anyway, it makes sense. Now I do see that removing the sorting would help move people over to using a library... is there a separate place where the library is being discussed/designed? Requirements being gathered?
-serge
Hi Serge and Tejun,
Quoted 19 lines · Tejun Heo
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
Quoted 3 lines
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Thanks both of you for the response :) So I'll put it aside for now.
Regards,
-Jeff
Hi Serge and Tejun,
Quoted 19 lines · Tejun Heo
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
Quoted 3 lines
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Thanks both of you for the response :) So I'll put it aside for now.
Regards,
-Jeff
Hello, guys.
Quoted 10 lines · Serge Hallyn
Quoted 9 lines
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
Quoted 5 lines
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
I want it gone but last time I pinged there were some people depending on it, so I don't think I can. I think we'll have to put it on ice for now at least. :(
Thanks.
Signature · 1 line
tejun
Quoted 26 lines · Jeff Liu
Quoted 14 lines · Serge Hallyn
Quoted 13 lines · Jeff Liu
Quoted 12 lines · Serge Hallyn
Quoted 11 lines · Jeff Liu
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 atDocumentation/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.Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
This patch remove the sorting function to make the default behavior of pid list be consistent with the document.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
Quoted 6 lines
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in
cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
Yeah, I figured, and since he also wants to move to a place where cgroups are always manipulated using a library, not by hand, it would make sense to get rid of the overhead there. I just don't really like either one of those :) And in the list of things to clean up in cgroups, this seems like one that can wait.
-serge
Quoted 14 lines · Serge Hallyn
Quoted 13 lines · Jeff Liu
Quoted 12 lines · Serge Hallyn
Quoted 11 lines · Jeff Liu
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 atDocumentation/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.
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
This patch remove the sorting function to make the default behavior of pid list be consistent with the document.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
I did this for two reasons, one is for the documents, another is per Cgroup TODO list from Tejun -- https://lkml.org/lkml/2012/9/13/542
. Misc issues
* Sort & unique when listing tasks. Even the documentation says it doesn't happen but we have a good hunk of code doing it in cgroup.c. I'm gonna rip it out at some point. Again, if you don't like it, scream.
So that we can clean out around 70-lines source code as well as shrink the cgroup.o a bit.
Thanks,
-Jeff
Quoted 7 lines · Jeff Liu
Quoted 6 lines · Serge Hallyn
Quoted 5 lines · Jeff Liu
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 atDocumentation/cgroups/cgroups.txt.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
- 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.
...
I've gotta say, as someone who tends to play with those files by hand, I don't mind not having to type | sort | uniq every time.
-serge
Quoted 81 lines · Serge Hallyn
Quoted 80 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +0
-6@@ -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);
Quoted 8 lines
kernel/cgroup.c static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type, · line 3462
Quoted 6 lines
kernel/cgroup.c +1
-2length = 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
Quoted 78 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +0
-6@@ -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
kernel/cgroup.c static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type, · line 3462
Quoted 8 lines
kernel/cgroup.c +1
-2@@ -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
-serge
Quoted 78 lines · Jeff Liu
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 atDocumentation/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 2942Patch 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 2781Signed-off-by Jie Liu <[email protected]>--- kernel/cgroup.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)kernel/cgroup.c +0
-6@@ -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
kernel/cgroup.c static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type, · line 3462
Quoted 8 lines
kernel/cgroup.c +1
-2@@ -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
-serge