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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux