kernfs, cgroup: reimplement "cgroup.procs" reading for v2
Tejun Heo
maintainerPATCH 1/1 kernfs, cgroup: reimplement "cgroup.procs" reading for v2
On cgroup v1, the pid listings in "cgroup.procs" and "tasks" are sorted which adds a lot of complications and overhead. v2 doesn't have such requirement and has been intentionally using a modified sorting order so that the output doesn't look sorted to users.
This patchset re-implements "cgroup.procs" reading for v2 which simply keeps a css_task_iter open while the file is being read. Keeping the iterator open makes it unnecessary to skip to the right position on each read segment and associated errors - e.g. incorrectly skipping over pids because earlier pids disappeared between the reads.
Using persistent iterator across multiple read calls requires ->release() callback to clean it up. kernfs operations ->open/release() are added and piped through cftype.
This patchset contains the following five patches.
0001-kernfs-make-kernfs_open_file-mmapped-a-bitfield.patch 0002-kernfs-add-kernfs_ops-open-release-callbacks.patch 0003-cgroup-add-cftype-open-release-callbacks.patch 0004-cgroup-reimplement-reading-cgroup.procs-on-cgroup-v2.patch 0005-cgroup-remove-cgroup_pid_fry-and-friends.patch
0001 is a misc kernfs patch and 0002 adds ->open/release() to kernfs. 0003 pipes ->open/release() through cftype. 0004 implements the new cgroup.procs for v2 and 0005 removes the now unused sort order frying logic.
Greg, would it be okay to route the kernfs patches through cgroup/for-4.11?
The patches are also available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git review-cgroup2-procs
diffstat follows. Thanks.
fs/kernfs/dir.c | 2 fs/kernfs/file.c | 53 +++++++++++++++-- fs/kernfs/kernfs-internal.h | 2 include/linux/cgroup-defs.h | 3 + include/linux/kernfs.h | 12 +++- kernel/cgroup.c | 130 +++++++++++++++++++++++++++++--------------- 6 files changed, 148 insertions(+), 54 deletions(-)
--
tejun
Tejun Heo
maintainerPATCH 1/5 kernfs: make kernfs_open_file->mmapped a bitfield
More kernfs_open_file->mutex synchronized flags are planned to be added. Convert ->mmapped to a bitfield in preparation.
While at it, make kernfs_fop_mmap() use "true" instead of "1" on ->mmapped.
--- fs/kernfs/file.c | 2 +- include/linux/kernfs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
fs/kernfs/file.c +1 -1
@@ -516,7 +516,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma) goto out_put; rc =0 ; - of->mmapped =1 ; + of->mmapped = true; of->vm_ops = vma->vm_ops; vma->vm_ops = &kernfs_vm_ops; out_put:
include/linux/kernfs.h +1 -1
@@ -185,7 +185,7 @@ struct kernfs_open_file {char *prealloc_buf;size_t atomic_write_len; -bool mmapped; + bool mmapped:1;const struct vm_operations_struct *vm_ops; };
Signature · 1 line
2.9.3
Tejun Heo
maintainerPATCH 2/5 kernfs: add kernfs_ops->open/release() callbacks
In reply to Tejun HeoAdd ->open/release() methods to kernfs_ops. ->open() is called when the file is opened and ->release() when the file is either released or severed. These callbacks can be used, for example, to manage persistent caching objects over multiple seq_file iterations.
--- fs/kernfs/dir.c | 2 +- fs/kernfs/file.c | 51 +++++++++++++++++++++++++++++++++++++++------ fs/kernfs/kernfs-internal.h | 2 +- include/linux/kernfs.h | 10 +++++++++ 4 files changed, 57 insertions(+), 8 deletions(-)
fs/kernfs/dir.c +1 -1
@@ -468,7 +468,7 @@ static void kernfs_drain(struct kernfs_node *kn)rwsem_release (&kn->dep_map,1 , _RET_IP_); } -kernfs_unmap_bin_file (kn); +kernfs_drain_open_files (kn);mutex_lock (&kernfs_mutex); }
fs/kernfs/file.c +45 -6
104 lines not on this page · raw message
fs/kernfs/kernfs-internal.h +1 -1
@@ -104,7 +104,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,*/ extern const struct file_operations kernfs_file_fops; -void kernfs_unmap_bin_file (struct kernfs_node *kn); +void kernfs_drain_open_files (struct kernfs_node *kn);/* * symlink.c
include/linux/kernfs.h +10 -0
@@ -46,6 +46,7 @@ enum kernfs_node_flag {KERNFS_SUICIDAL =0x0400 ,KERNFS_SUICIDED =0x0800 ,KERNFS_EMPTY_DIR =0x1000 , + KERNFS_HAS_RELEASE = 0x2000, };/* @flags for kernfs_create_root() */
@@ -175,6 +176,7 @@ struct kernfs_open_file {/* published fields */ struct kernfs_node *kn;struct file *file; +struct seq_file *seq_file;void *priv;/* private fields, do not use outside kernfs proper */
@@ -186,11 +188,19 @@ struct kernfs_open_file {size_t atomic_write_len; bool mmapped:1; + bool released:1;const struct vm_operations_struct *vm_ops; };struct kernfs_ops {/* +* Optional open/release methods. Both are called with +* @of->seq_file populated. +*/ +int (*open)(struct kernfs_open_file *of); +void (*release)(struct kernfs_open_file *of); + +/* * Read is handled by either seq_file or raw_read(). * * If seq_show() is present, seq_file path is active. Other seq
Signature · 1 line
2.9.3
Tejun Heo
maintainerPATCH 3/5 cgroup add cftype->open/release() callbacks
In reply to Tejun HeoPipe the newly added kernfs->open/release() callbacks through cftype. While at it, as cleanup operations now can be performed from ->release() instead of ->seq_stop(), make the latter optional.
--- include/linux/cgroup-defs.h | 3 +++ kernel/cgroup.c | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)
include/linux/cgroup-defs.h +3 -0
@@ -388,6 +388,9 @@ struct cftype {struct list_head node;/* anchored at ss->cfts */ struct kernfs_ops *kf_ops; +int (*open)(struct kernfs_open_file *of); +void (*release)(struct kernfs_open_file *of); +/* * read_u64() is a shortcut for the common case of returning a * single integer. Use it in place of read()
kernel/cgroup.c +23 -1
@@ -3503,6 +3503,23 @@ static int cgroup_events_show(struct seq_file *seq, void *v)return 0 ; } +static int cgroup_file_open (struct kernfs_open_file *of) +{ +struct cftype *cft = of->kn->priv; + +if (cft->open) +return cft->open (of); +return 0 ; +} + +static void cgroup_file_release (struct kernfs_open_file *of) +{ +struct cftype *cft = of->kn->priv; + +if (cft->release) + cft->release (of); +} +static ssize_t cgroup_file_write (struct kernfs_open_file *of,char *buf,size_t nbytes,loff_t off) {
@@ -3553,7 +3570,8 @@ static void *cgroup_seqfile_next(struct seq_file *seq, void *v, loff_t *ppos)static void cgroup_seqfile_stop (struct seq_file *seq,void *v) { -seq_cft (seq)->seq_stop (seq, v); +if (seq_cft (seq)->seq_stop) +seq_cft (seq)->seq_stop (seq, v); }static int cgroup_seqfile_show (struct seq_file *m,void *arg)
@@ -3575,12 +3593,16 @@ static int cgroup_seqfile_show(struct seq_file *m, void *arg)static struct kernfs_ops cgroup_kf_single_ops = { .atomic_write_len =PAGE_SIZE , + .open = cgroup_file_open, + .release = cgroup_file_release, .write = cgroup_file_write, .seq_show = cgroup_seqfile_show, };static struct kernfs_ops cgroup_kf_ops = { .atomic_write_len =PAGE_SIZE , + .open = cgroup_file_open, + .release = cgroup_file_release, .write = cgroup_file_write, .seq_start = cgroup_seqfile_start, .seq_next = cgroup_seqfile_next,
Signature · 1 line
2.9.3
Tejun Heo
maintainerPATCH 4/5 cgroup: reimplement reading "cgroup.procs" on cgroup v2
In reply to Tejun HeoOn v1, "tasks" and "cgroup.procs" are expected to be sorted which makes the implementation expensive and unnecessarily complicated involving result cache management.
v2 doesn't have the sorting requirement, so it can just iterate and print processes one by one. seq_files are either read sequentially or reset to position zero, so the implementation doesn't even need to worry about seeking.
This keeps the css_task_iter across multiple read(2) calls and migrations of new processes always append won't miss processes which are newly migrated in before each read(2).
--- kernel/cgroup.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 5 deletions(-)
kernel/cgroup.c +58 -5
@@ -4426,6 +4426,60 @@ int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from)return ret; } +static void cgroup_procs_release (struct kernfs_open_file *of) +{ +if (of->priv) { +css_task_iter_end (of->priv); +kfree (of->priv); + } +} + +static void *cgroup_procs_next (struct seq_file *s,void *v,loff_t *pos) +{ +struct kernfs_open_file *of = s->private; +struct css_task_iter *it = of->priv; +struct task_struct *task; + +do { + task =css_task_iter_next (it); + }while (task && !thread_group_leader (task)); + +return task; +} + +static void *cgroup_procs_start (struct seq_file *s,loff_t *pos) +{ +struct kernfs_open_file *of = s->private; +struct cgroup *cgrp =seq_css (s)->cgroup; +struct css_task_iter *it = of->priv; + +/* +* When a seq_file is seeked, it's always traversed sequentially +* from position 0, so we can simply keep iterating on !0 *pos. +*/ +if (!it) { +if (WARN_ON_ONCE ((*pos)++)) +return ERR_PTR (-EINVAL ); + + it =kzalloc (sizeof (*it),GFP_KERNEL ); +if (!it) +return ERR_PTR (-ENOMEM ); + of->priv = it; +css_task_iter_start (&cgrp->self, it); + }else if (!(*pos)++) { +css_task_iter_end (it); +css_task_iter_start (&cgrp->self, it); + } + +return cgroup_procs_next (s,NULL ,NULL ); +} + +static int cgroup_procs_show (struct seq_file *s,void *v) +{ +seq_printf (s,"%d\n" ,task_tgid_vnr (v)); +return 0 ; +} +/* * Stuff for reading the 'tasks'/'procs' files. *
@@ -4914,11 +4968,10 @@ static struct cftype cgroup_dfl_base_files[] = { { .name = "cgroup.procs", .file_offset = offsetof(struct cgroup, procs_file), - .seq_start = cgroup_pidlist_start, - .seq_next = cgroup_pidlist_next, - .seq_stop = cgroup_pidlist_stop, - .seq_show = cgroup_pidlist_show, - .private = CGROUP_FILE_PROCS, + .release = cgroup_procs_release, + .seq_start = cgroup_procs_start, + .seq_next = cgroup_procs_next, + .seq_show = cgroup_procs_show, .write = cgroup_procs_write, }, {
Signature · 1 line
2.9.3
Tejun Heo
maintainerPATCH 5/5 cgroup: remove cgroup_pid_fry() and friends
In reply to Tejun Heocgroup_pid_fry() was added to mangle cgroup.procs pid listing order on v2 to make it clear that the output is not sorted. Now that v2 now uses a separate "cgroup.procs" read method, this is no longer used. Remove it.
--- kernel/cgroup.c | 43 +++++-------------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-)
kernel/cgroup.c +5 -38
@@ -4618,42 +4618,12 @@ static int pidlist_uniq(pid_t *list, int length)* sorted by task pointer. As pidlists can be fairly large, allocating one * per open file is dangerous, so cgroup had to implement shared pool of * pidlists keyed by cgroup and namespace. -* -* All this extra complexity was caused by the original implementation -* committing to an entirely unnecessary property. In the long term, we -* want to do away with it. Explicitly scramble sort order if on the -* default hierarchy so that no such expectation exists in the new -* interface. -* -* Scrambling is done by swapping every two consecutive bits, which is -* non-identity one-to-one mapping which disturbs sort order sufficiently. */ -static pid_t pid_fry (pid_t pid) -{ -unsigned a = pid &0x55555555 ; -unsigned b = pid &0xAAAAAAAA ; - -return (a <<1 ) | (b >>1 ); -} - -static pid_t cgroup_pid_fry (struct cgroup *cgrp,pid_t pid) -{ -if (cgroup_on_dfl (cgrp)) -return pid_fry (pid); -else -return pid; -} -static int cmppid (const void *a,const void *b) {return *(pid_t *)a - *(pid_t *)b; } -static int fried_cmppid (const void *a,const void *b) -{ -return pid_fry (*(pid_t *)a) -pid_fry (*(pid_t *)b); -} -static struct cgroup_pidlist *cgroup_pidlist_find (struct cgroup *cgrp,enum cgroup_filetype type) {
@@ -4741,10 +4711,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,css_task_iter_end (&it); length = n;/* now sort & (if procs) strip out duplicates */ -if (cgroup_on_dfl (cgrp)) -sort (array, length,sizeof (pid_t), fried_cmppid,NULL ); -else -sort (array, length,sizeof (pid_t), cmppid,NULL ); +sort (array, length,sizeof (pid_t), cmppid,NULL );if (type ==CGROUP_FILE_PROCS ) length =pidlist_uniq (array, length);
@@ -4876,10 +4843,10 @@ static void *cgroup_pidlist_start(struct seq_file *s, loff_t *pos)while (index < end) {int mid = (index + end) /2 ; -if (cgroup_pid_fry (cgrp, l->list[mid]) == pid) { +if (l->list[mid] == pid) { index = mid;break ; - }else if (cgroup_pid_fry (cgrp, l->list[mid]) <= pid) + }else if (l->list[mid] <= pid) index = mid +1 ;else end = mid;
@@ -4890,7 +4857,7 @@ static void *cgroup_pidlist_start(struct seq_file *s, loff_t *pos)return NULL ;/* Update the abstract position to be the actual pid that we found */ iter = l->list + index; - *pos =cgroup_pid_fry (cgrp, *iter); + *pos = *iter;return iter; }
@@ -4919,7 +4886,7 @@ static void *cgroup_pidlist_next(struct seq_file *s, void *v, loff_t *pos)if (p >= end) {return NULL ; }else { - *pos =cgroup_pid_fry (seq_css (s)->cgroup, *p); + *pos = *p;return p; } }
Signature · 1 line
2.9.3
Quoted 23 lines · Tejun Heo
On cgroup v1, the pid listings in "cgroup.procs" and "tasks" are sorted which adds a lot of complications and overhead. v2 doesn't have such requirement and has been intentionally using a modified sorting order so that the output doesn't look sorted to users.
This patchset re-implements "cgroup.procs" reading for v2 which simply keeps a
css_task_iteropen while the file is being read. Keeping the iterator open makes it unnecessary to skip to the right position on each read segment and associated errors - e.g. incorrectly skipping over pids because earlier pids disappeared between the reads.Using persistent iterator across multiple read calls requires ->release() callback to clean it up. kernfs operations ->open/release() are added and piped through cftype.
This patchset contains the following five patches.
0001-kernfs-make-kernfs_open_file-mmapped-a-bitfield.patch 0002-kernfs-add-kernfs_ops-open-release-callbacks.patch 0003-cgroup-add-cftype-open-release-callbacks.patch 0004-cgroup-reimplement-reading-cgroup.procs-on-cgroup-v2.patch 0005-cgroup-remove-cgroup_pid_fry-and-friends.patch
Quoted 7 lines
0001 is a misc kernfs patch and 0002 adds ->open/release() to kernfs. 0003 pipes ->open/release() through cftype. 0004 implements the new cgroup.procs for v2 and 0005 removes the now unused sort order frying logic.
Greg, would it be okay to route the kernfs patches through cgroup/for-4.11?
No objection from me at all:
Quoted 37 lines · Tejun Heo
On cgroup v1, the pid listings in "cgroup.procs" and "tasks" are sorted which adds a lot of complications and overhead. v2 doesn't have such requirement and has been intentionally using a modified sorting order so that the output doesn't look sorted to users.
This patchset re-implements "cgroup.procs" reading for v2 which simply keeps a
css_task_iteropen while the file is being read. Keeping the iterator open makes it unnecessary to skip to the right position on each read segment and associated errors - e.g. incorrectly skipping over pids because earlier pids disappeared between the reads.Using persistent iterator across multiple read calls requires ->release() callback to clean it up. kernfs operations ->open/release() are added and piped through cftype.
This patchset contains the following five patches.
0001-kernfs-make-kernfs_open_file-mmapped-a-bitfield.patch 0002-kernfs-add-kernfs_ops-open-release-callbacks.patch 0003-cgroup-add-cftype-open-release-callbacks.patch 0004-cgroup-reimplement-reading-cgroup.procs-on-cgroup-v2.patch 0005-cgroup-remove-cgroup_pid_fry-and-friends.patch
0001 is a misc kernfs patch and 0002 adds ->open/release() to kernfs. 0003 pipes ->open/release() through cftype. 0004 implements the new cgroup.procs for v2 and 0005 removes the now unused sort order frying logic.
Greg, would it be okay to route the kernfs patches through cgroup/for-4.11?
The patches are also available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.gitreview-cgroup2-procsdiffstat follows. Thanks.
Quoted 8 lines
fs/kernfs/dir.c | 2 fs/kernfs/file.c | 53 +++++++++++++++-- fs/kernfs/kernfs-internal.h | 2 include/linux/cgroup-defs.h | 3 + include/linux/kernfs.h | 12 +++- kernel/cgroup.c | 130 +++++++++++++++++++++++++++++--------------- 6 files changed, 148 insertions(+), 54 deletions(-)
Quoted 11 lines · Tejun Heo
On cgroup v1, the pid listings in "cgroup.procs" and "tasks" are sorted which adds a lot of complications and overhead. v2 doesn't have such requirement and has been intentionally using a modified sorting order so that the output doesn't look sorted to users.
This patchset re-implements "cgroup.procs" reading for v2 which simply keeps a
css_task_iteropen while the file is being read. Keeping the iterator open makes it unnecessary to skip to the right position on each read segment and associated errors - e.g. incorrectly skipping over pids because earlier pids disappeared between the reads.
Quoted 3 lines
Using persistent iterator across multiple read calls requires ->release() callback to clean it up. kernfs operations ->open/release() are added and piped through cftype.
Applied to cgroup/for-4.11.
Signature · 1 line
tejun