[PATCH] cgroup: minor optimization around the usage of cur_tasks_head

Yafang Shao
2022-01-26T14:17:05Z

Recently there was an issue occurred on our production envrionment with a
very old kernel version 4.19. That issue can be fixed by upstream
commit 9c974c772464 ("cgroup: Iterate tasks that did not finish do_exit()")

When I was trying to fix that issue on our production environment, I found
we can create a hotfix with a simplified version of the commit -

As the usage of cur_tasks_head is within the function
css_task_iter_advance(), we can make it as a local variable. That could
make it more clear and easier to understand. Another benefit is we don't
need to carry it in css_task_iter.

Signed-off-by: Yafang Shao <laoar.shao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Michal Koutn√Ω <mkoutny-IBi9RG/b67k@public.gmane.org>
---
include/linux/cgroup.h | 1 -
kernel/cgroup/cgroup.c | 29 ++++++++++++++++-------------
2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 75c151413fda..f619a92d0fa0 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -61,7 +61,6 @@ struct css_task_iter {

struct list_head *task_pos;

- struct list_head *cur_tasks_head;
struct css_set *cur_cset;
struct css_set *cur_dcset;
struct task_struct *cur_task;
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 919194de39c8..ffb0c863b97a 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4545,10 +4545,12 @@ static struct css_set *css_task_iter_next_css_set(struct css_task_iter *it)
/**
* css_task_iter_advance_css_set - advance a task iterator to the next css_set
* @it: the iterator to advance
+ * @cur_tasks_head: head of current tasks
*
* Advance @it to the next css_set to walk.
*/
-static void css_task_iter_advance_css_set(struct css_task_iter *it)
+static void css_task_iter_advance_css_set(struct css_task_iter *it,
+ struct list_head **cur_tasks_head)
{
struct css_set *cset;

@@ -4557,13 +4559,13 @@ static void css_task_iter_advance_css_set(struct css_task_iter *it)
/* Advance to the next non-empty css_set and find first non-empty tasks list*/
while ((cset = css_task_iter_next_css_set(it))) {
if (!list_empty(&cset->tasks)) {
- it->cur_tasks_head = &cset->tasks;
+ *cur_tasks_head = &cset->tasks;
break;
} else if (!list_empty(&cset->mg_tasks)) {
- it->cur_tasks_head = &cset->mg_tasks;
+ *cur_tasks_head = &cset->mg_tasks;
break;
} else if (!list_empty(&cset->dying_tasks)) {
- it->cur_tasks_head = &cset->dying_tasks;
+ *cur_tasks_head = &cset->dying_tasks;
break;
}
}
@@ -4571,7 +4573,7 @@ static void css_task_iter_advance_css_set(struct css_task_iter *it)
it->task_pos = NULL;
return;
}
- it->task_pos = it->cur_tasks_head->next;
+ it->task_pos = (*cur_tasks_head)->next;

/*
* We don't keep css_sets locked across iteration steps and thus
@@ -4610,6 +4612,7 @@ static void css_task_iter_skip(struct css_task_iter *it,

static void css_task_iter_advance(struct css_task_iter *it)
{
+ struct list_head *cur_tasks_head = NULL;
struct task_struct *task;

lockdep_assert_held(&css_set_lock);
@@ -4626,18 +4629,18 @@ static void css_task_iter_advance(struct css_task_iter *it)
it->task_pos = it->task_pos->next;

if (it->task_pos == &it->cur_cset->tasks) {
- it->cur_tasks_head = &it->cur_cset->mg_tasks;
- it->task_pos = it->cur_tasks_head->next;
+ cur_tasks_head = &it->cur_cset->mg_tasks;
+ it->task_pos = cur_tasks_head->next;
}
if (it->task_pos == &it->cur_cset->mg_tasks) {
- it->cur_tasks_head = &it->cur_cset->dying_tasks;
- it->task_pos = it->cur_tasks_head->next;
+ cur_tasks_head = &it->cur_cset->dying_tasks;
+ it->task_pos = cur_tasks_head->next;
}
if (it->task_pos == &it->cur_cset->dying_tasks)
- css_task_iter_advance_css_set(it);
+ css_task_iter_advance_css_set(it, &cur_tasks_head);
} else {
/* called from start, proceed to the first cset */
- css_task_iter_advance_css_set(it);
+ css_task_iter_advance_css_set(it, &cur_tasks_head);
}

if (!it->task_pos)
@@ -4651,12 +4654,12 @@ static void css_task_iter_advance(struct css_task_iter *it)
goto repeat;

/* and dying leaders w/o live member threads */
- if (it->cur_tasks_head == &it->cur_cset->dying_tasks &&
+ if (cur_tasks_head == &it->cur_cset->dying_tasks &&
!atomic_read(&task->signal->live))
goto repeat;
} else {
/* skip all dying ones */
- if (it->cur_tasks_head == &it->cur_cset->dying_tasks)
+ if (cur_tasks_head == &it->cur_cset->dying_tasks)
goto repeat;
}
}
--
2.17.1