Hello Vasily.
On Sun, Jan 26, 2020 at 01:45:45PM +0300, Vasily Averin <vvs@xxxxxxxxxxxxx> wrote:
> In Aug 2018 NeilBrown noticed
> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
> "Some ->next functions do not increment *pos when they return NULL...
> Note that such ->next functions are buggy and should be fixed.
> A simple demonstration is
Thanks for bringing this up.
I agree with your changes, however, I have some suggestions:
1) squash [PATCH 2/2] "cgroup_procs_next should increase position index"
and [PATCH] "__cgroup_procs_start cleanup"
- make it clear in the commit message that it's fixing the small
buffer listing, it's not a mere cleanup(!)
2) for completeness, I propose squashing also this change (IOW,
cgroup_procs_start should only initialize the iterator, not move it):
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4617,7 +4617,7 @@ static void *__cgroup_procs_start(struct seq_file *s, loff_t *pos,
* from position 0, so we can simply keep iterating on !0 *pos.
*/
if (!it) {
- if (WARN_ON_ONCE((*pos)++))
+ if (WARN_ON_ONCE(*pos))
return ERR_PTR(-EINVAL);
it = kzalloc(sizeof(*it), GFP_KERNEL);
@@ -4625,7 +4625,7 @@ static void *__cgroup_procs_start(struct seq_file *s, loff_t *pos,
return ERR_PTR(-ENOMEM);
of->priv = it;
css_task_iter_start(&cgrp->self, iter_flags, it);
- } else if (!(*pos)++) {
+ } else if (!(*pos)) {
css_task_iter_end(it);
css_task_iter_start(&cgrp->self, iter_flags, it);
} else
3) I was not able to reproduce the corrupted listing into small buffer
on v1 hierarchy, i.e. the [PATCH 1/2] "cgroup_pidlist_next should update
position index" log message should just explain the change is to
satisfy seq_file iterator requirements.
I can send my complete diffs if the suggestions are unclear.
Michal
P.S. I really recommend using `git send-email` for sending out the
patches, it makes mail threading more readable.
Attachment:
signature.asc
Description: Digital signature
