From: Michal Hocko <mhocko@suse.cz>
To: Tejun Heo <tj@kernel.org>
Cc: lizefan@huawei.com, containers@lists.linux-foundation.org,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
Matt Helsley <matthltc@us.ibm.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Balbir Singh <bsingharora@gmail.com>
Subject: Re: [PATCH 18/23] cgroup: make cgroup_task_iter remember the cgroup being iterated
Date: Fri, 2 Aug 2013 15:38:32 +0200 [thread overview]
Message-ID: <20130802133832.GF25432@dhcp22.suse.cz> (raw)
In-Reply-To: <1375393801-4817-19-git-send-email-tj@kernel.org>
On Thu 01-08-13 17:49:56, Tejun Heo wrote:
> Currently all cgroup_task_iter functions require @cgrp to be passed
> in, which is superflous and increases chance of usage error. Make
> cgroup_task_iter remember the cgroup being iterated and drop @cgrp
> argument from next and end functions.
>
> This patch doesn't introduce any behavior differences.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Matt Helsley <matthltc@us.ibm.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Balbir Singh <bsingharora@gmail.com>
For memcg part
Acked-by: Michal Hocko <mhocko@suse.cz>
> ---
> include/linux/cgroup.h | 6 +++---
> kernel/cgroup.c | 32 +++++++++++++++-----------------
> kernel/cgroup_freezer.c | 12 ++++++------
> mm/memcontrol.c | 6 +++---
> 4 files changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 4478336..2b10152 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -892,14 +892,14 @@ css_next_descendant_post(struct cgroup_subsys_state *pos,
>
> /* A cgroup_task_iter should be treated as an opaque object */
> struct cgroup_task_iter {
> + struct cgroup *origin_cgrp;
> struct list_head *cset_link;
> struct list_head *task;
> };
>
> void cgroup_task_iter_start(struct cgroup *cgrp, struct cgroup_task_iter *it);
> -struct task_struct *cgroup_task_iter_next(struct cgroup *cgrp,
> - struct cgroup_task_iter *it);
> -void cgroup_task_iter_end(struct cgroup *cgrp, struct cgroup_task_iter *it);
> +struct task_struct *cgroup_task_iter_next(struct cgroup_task_iter *it);
> +void cgroup_task_iter_end(struct cgroup_task_iter *it);
> int cgroup_scan_tasks(struct cgroup_scanner *scan);
> int cgroup_attach_task_all(struct task_struct *from, struct task_struct *);
> int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from);
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 7a4f89b..7adaaa6 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -3198,13 +3198,11 @@ EXPORT_SYMBOL_GPL(css_next_descendant_post);
>
> /**
> * cgroup_advance_task_iter - advance a task itererator to the next css_set
> - * @cgrp: the cgroup to walk tasks of
> * @it: the iterator to advance
> *
> * Advance @it to the next css_set to walk.
> */
> -static void cgroup_advance_task_iter(struct cgroup *cgrp,
> - struct cgroup_task_iter *it)
> +static void cgroup_advance_task_iter(struct cgroup_task_iter *it)
> {
> struct list_head *l = it->cset_link;
> struct cgrp_cset_link *link;
> @@ -3213,7 +3211,7 @@ static void cgroup_advance_task_iter(struct cgroup *cgrp,
> /* Advance to the next non-empty css_set */
> do {
> l = l->next;
> - if (l == &cgrp->cset_links) {
> + if (l == &it->origin_cgrp->cset_links) {
> it->cset_link = NULL;
> return;
> }
> @@ -3250,21 +3248,22 @@ void cgroup_task_iter_start(struct cgroup *cgrp, struct cgroup_task_iter *it)
> cgroup_enable_task_cg_lists();
>
> read_lock(&css_set_lock);
> +
> + it->origin_cgrp = cgrp;
> it->cset_link = &cgrp->cset_links;
> - cgroup_advance_task_iter(cgrp, it);
> +
> + cgroup_advance_task_iter(it);
> }
>
> /**
> * cgroup_task_iter_next - return the next task for the iterator
> - * @cgrp: the cgroup to walk tasks of
> * @it: the task iterator being iterated
> *
> * The "next" function for task iteration. @it should have been
> * initialized via cgroup_task_iter_start(). Returns NULL when the
> * iteration reaches the end.
> */
> -struct task_struct *cgroup_task_iter_next(struct cgroup *cgrp,
> - struct cgroup_task_iter *it)
> +struct task_struct *cgroup_task_iter_next(struct cgroup_task_iter *it)
> {
> struct task_struct *res;
> struct list_head *l = it->task;
> @@ -3282,7 +3281,7 @@ struct task_struct *cgroup_task_iter_next(struct cgroup *cgrp,
> * We reached the end of this task list - move on to the
> * next cgrp_cset_link.
> */
> - cgroup_advance_task_iter(cgrp, it);
> + cgroup_advance_task_iter(it);
> } else {
> it->task = l;
> }
> @@ -3291,12 +3290,11 @@ struct task_struct *cgroup_task_iter_next(struct cgroup *cgrp,
>
> /**
> * cgroup_task_iter_end - finish task iteration
> - * @cgrp: the cgroup to walk tasks of
> * @it: the task iterator to finish
> *
> * Finish task iteration started by cgroup_task_iter_start().
> */
> -void cgroup_task_iter_end(struct cgroup *cgrp, struct cgroup_task_iter *it)
> +void cgroup_task_iter_end(struct cgroup_task_iter *it)
> __releases(css_set_lock)
> {
> read_unlock(&css_set_lock);
> @@ -3402,7 +3400,7 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
> */
> heap->size = 0;
> cgroup_task_iter_start(scan->cgrp, &it);
> - while ((p = cgroup_task_iter_next(scan->cgrp, &it))) {
> + while ((p = cgroup_task_iter_next(&it))) {
> /*
> * Only affect tasks that qualify per the caller's callback,
> * if he provided one
> @@ -3435,7 +3433,7 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
> * the heap and wasn't inserted
> */
> }
> - cgroup_task_iter_end(scan->cgrp, &it);
> + cgroup_task_iter_end(&it);
>
> if (heap->size) {
> for (i = 0; i < heap->size; i++) {
> @@ -3657,7 +3655,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
> return -ENOMEM;
> /* now, populate the array */
> cgroup_task_iter_start(cgrp, &it);
> - while ((tsk = cgroup_task_iter_next(cgrp, &it))) {
> + while ((tsk = cgroup_task_iter_next(&it))) {
> if (unlikely(n == length))
> break;
> /* get tgid or pid for procs or tasks file respectively */
> @@ -3668,7 +3666,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
> if (pid > 0) /* make sure to only use valid results */
> array[n++] = pid;
> }
> - cgroup_task_iter_end(cgrp, &it);
> + cgroup_task_iter_end(&it);
> length = n;
> /* now sort & (if procs) strip out duplicates */
> sort(array, length, sizeof(pid_t), cmppid, NULL);
> @@ -3717,7 +3715,7 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)
> cgrp = dentry->d_fsdata;
>
> cgroup_task_iter_start(cgrp, &it);
> - while ((tsk = cgroup_task_iter_next(cgrp, &it))) {
> + while ((tsk = cgroup_task_iter_next(&it))) {
> switch (tsk->state) {
> case TASK_RUNNING:
> stats->nr_running++;
> @@ -3737,7 +3735,7 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)
> break;
> }
> }
> - cgroup_task_iter_end(cgrp, &it);
> + cgroup_task_iter_end(&it);
>
> err:
> return ret;
> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
> index c9177f8..e0ab9bf 100644
> --- a/kernel/cgroup_freezer.c
> +++ b/kernel/cgroup_freezer.c
> @@ -281,7 +281,7 @@ static void update_if_frozen(struct cgroup_subsys_state *css)
> /* are all tasks frozen? */
> cgroup_task_iter_start(css->cgroup, &it);
>
> - while ((task = cgroup_task_iter_next(css->cgroup, &it))) {
> + while ((task = cgroup_task_iter_next(&it))) {
> if (freezing(task)) {
> /*
> * freezer_should_skip() indicates that the task
> @@ -296,7 +296,7 @@ static void update_if_frozen(struct cgroup_subsys_state *css)
>
> freezer->state |= CGROUP_FROZEN;
> out_iter_end:
> - cgroup_task_iter_end(css->cgroup, &it);
> + cgroup_task_iter_end(&it);
> out_unlock:
> spin_unlock_irq(&freezer->lock);
> }
> @@ -327,9 +327,9 @@ static void freeze_cgroup(struct freezer *freezer)
> struct task_struct *task;
>
> cgroup_task_iter_start(cgroup, &it);
> - while ((task = cgroup_task_iter_next(cgroup, &it)))
> + while ((task = cgroup_task_iter_next(&it)))
> freeze_task(task);
> - cgroup_task_iter_end(cgroup, &it);
> + cgroup_task_iter_end(&it);
> }
>
> static void unfreeze_cgroup(struct freezer *freezer)
> @@ -339,9 +339,9 @@ static void unfreeze_cgroup(struct freezer *freezer)
> struct task_struct *task;
>
> cgroup_task_iter_start(cgroup, &it);
> - while ((task = cgroup_task_iter_next(cgroup, &it)))
> + while ((task = cgroup_task_iter_next(&it)))
> __thaw_task(task);
> - cgroup_task_iter_end(cgroup, &it);
> + cgroup_task_iter_end(&it);
> }
>
> /**
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 00b055d..5a5f4dc 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1804,7 +1804,7 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> struct task_struct *task;
>
> cgroup_task_iter_start(cgroup, &it);
> - while ((task = cgroup_task_iter_next(cgroup, &it))) {
> + while ((task = cgroup_task_iter_next(&it))) {
> switch (oom_scan_process_thread(task, totalpages, NULL,
> false)) {
> case OOM_SCAN_SELECT:
> @@ -1817,7 +1817,7 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> case OOM_SCAN_CONTINUE:
> continue;
> case OOM_SCAN_ABORT:
> - cgroup_task_iter_end(cgroup, &it);
> + cgroup_task_iter_end(&it);
> mem_cgroup_iter_break(memcg, iter);
> if (chosen)
> put_task_struct(chosen);
> @@ -1834,7 +1834,7 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
> get_task_struct(chosen);
> }
> }
> - cgroup_task_iter_end(cgroup, &it);
> + cgroup_task_iter_end(&it);
> }
>
> if (!chosen)
> --
> 1.8.3.1
>
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2013-08-02 13:38 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-01 21:49 [PATCHSET cgroup/for-3.12] cgroup: use cgroup_subsys_state as the primary subsystem interface handle Tejun Heo
2013-08-01 21:49 ` [PATCH 01/23] cgroup: s/cgroup_subsys_state/cgroup_css/ s/task_subsys_state/task_css/ Tejun Heo
2013-08-01 21:49 ` [PATCH 02/23] cpuset: drop "const" qualifiers from struct cpuset instances Tejun Heo
2013-08-01 21:49 ` [PATCH 03/23] netprio_cgroup: pass around @css instead of @cgroup and kill struct cgroup_netprio_state Tejun Heo
2013-08-01 22:07 ` David Miller
2013-08-02 11:42 ` Neil Horman
2013-08-01 21:49 ` [PATCH 04/23] hugetlb_cgroup: pass around @hugetlb_cgroup instead of @cgroup Tejun Heo
2013-08-02 4:35 ` Aneesh Kumar K.V
2013-08-02 13:10 ` Michal Hocko
2013-08-01 21:49 ` [PATCH 05/23] cgroup: add subsystem pointer to cgroup_subsys_state Tejun Heo
2013-08-01 21:49 ` [PATCH 06/23] cgroup: add/update accessors which obtain subsys specific data from css Tejun Heo
2013-08-01 21:49 ` [PATCH 07/23] cgroup: add css_parent() Tejun Heo
2013-08-01 21:49 ` [PATCH 08/23] cgroup: pass around cgroup_subsys_state instead of cgroup in subsystem methods Tejun Heo
2013-08-02 3:54 ` Li Zefan
2013-08-02 19:36 ` Tejun Heo
2013-08-02 4:02 ` Li Zefan
2013-08-02 19:41 ` Tejun Heo
2013-08-02 13:19 ` Michal Hocko
2013-08-02 13:43 ` Michal Hocko
2013-08-02 19:52 ` Tejun Heo
2013-08-02 19:38 ` Tejun Heo
2013-08-02 20:24 ` [PATCH v2 " Tejun Heo
2013-08-06 7:19 ` Daniel Wagner
2013-08-05 12:44 ` [PATCH " Vivek Goyal
2013-08-05 17:57 ` Aristeu Rozanski
2013-08-01 21:49 ` [PATCH 09/23] cgroup: add subsys backlink pointer to cftype Tejun Heo
2013-08-05 12:49 ` Vivek Goyal
2013-08-01 21:49 ` [PATCH 10/23] cgroup: pin cgroup_subsys_state when opening a cgroupfs file Tejun Heo
2013-08-01 21:49 ` [PATCH 11/23] cgroup: add cgroup->dummy_css Tejun Heo
2013-08-01 21:49 ` [PATCH 12/23] cgroup: pass around cgroup_subsys_state instead of cgroup in file methods Tejun Heo
2013-08-02 13:27 ` Michal Hocko
2013-08-05 14:19 ` Vivek Goyal
2013-08-05 18:04 ` Aristeu Rozanski
2013-08-06 6:48 ` Daniel Wagner
2013-08-01 21:49 ` [PATCH 13/23] cgroup: convert cgroup_next_sibling() to cgroup_next_child() Tejun Heo
2013-08-01 21:49 ` [PATCH 14/23] cgroup: always use cgroup_next_child() to walk the children list Tejun Heo
2013-08-01 21:49 ` [PATCH 15/23] cgroup: make hierarchy iterators deal with cgroup_subsys_state instead of cgroup Tejun Heo
2013-08-02 13:32 ` Michal Hocko
2013-08-05 14:25 ` Vivek Goyal
2013-08-05 18:10 ` Aristeu Rozanski
2013-08-01 21:49 ` [PATCH 16/23] cgroup: relocate cgroup_advance_iter() Tejun Heo
2013-08-02 3:25 ` Li Zefan
2013-08-02 19:35 ` Tejun Heo
2013-08-01 21:49 ` [PATCH 17/23] cgroup: rename cgroup_iter to cgroup_task_iter Tejun Heo
2013-08-02 13:35 ` Michal Hocko
2013-08-01 21:49 ` [PATCH 18/23] cgroup: make cgroup_task_iter remember the cgroup being iterated Tejun Heo
2013-08-02 13:38 ` Michal Hocko [this message]
2013-08-01 21:49 ` [PATCH 19/23] cgroup: remove struct cgroup_scanner Tejun Heo
2013-08-01 21:49 ` [PATCH 20/23] cgroup: make task iterators deal with cgroup_subsys_state instead of cgroup Tejun Heo
2013-08-02 13:40 ` Michal Hocko
2013-08-01 21:49 ` [PATCH 21/23] cgroup: make cftype->[un]register_event() " Tejun Heo
2013-08-02 4:08 ` Li Zefan
2013-08-02 19:44 ` Tejun Heo
2013-08-02 13:42 ` Michal Hocko
2013-08-02 20:24 ` [PATCH v2 " Tejun Heo
2013-08-01 21:50 ` [PATCH 22/23] cgroup: make cgroup_taskset " Tejun Heo
2013-08-06 6:53 ` Daniel Wagner
2013-08-01 21:50 ` [PATCH 23/23] cgroup: unexport cgroup_css() Tejun Heo
2013-08-02 3:24 ` [PATCHSET cgroup/for-3.12] cgroup: use cgroup_subsys_state as the primary subsystem interface handle Li Zefan
2013-08-09 0:12 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130802133832.GF25432@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=bsingharora@gmail.com \
--cc=cgroups@vger.kernel.org \
--cc=containers@lists.linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=matthltc@us.ibm.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®