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@xxxxxxxxx> Cc: Michal Koutný <mkoutny@xxxxxxxx> --- 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