From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751330AbdJDUKs (ORCPT ); Wed, 4 Oct 2017 16:10:48 -0400 Received: from mail-pg0-f50.google.com ([74.125.83.50]:49790 "EHLO mail-pg0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751132AbdJDUKq (ORCPT ); Wed, 4 Oct 2017 16:10:46 -0400 X-Google-Smtp-Source: AOwi7QCIKC2bzgwPNhb9WNQMe+ojSf2pFoB3d9GNviHUi2cQBt3V/QmeAVEahU68YHA3liIKrvsV0A== Date: Wed, 4 Oct 2017 13:10:44 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Roman Gushchin cc: linux-mm@kvack.org, Vladimir Davydov , Johannes Weiner , Tetsuo Handa , Andrew Morton , Tejun Heo , kernel-team@fb.com, cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [v10 2/6] mm: implement mem_cgroup_scan_tasks() for the root memory cgroup In-Reply-To: <20171004154638.710-3-guro@fb.com> Message-ID: References: <20171004154638.710-1-guro@fb.com> <20171004154638.710-3-guro@fb.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 4 Oct 2017, Roman Gushchin wrote: > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index d5f3a62887cf..b4de17a78dc1 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -917,7 +917,8 @@ static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg) > * value, the function breaks the iteration loop and returns the value. > * Otherwise, it will iterate over all tasks and return 0. > * > - * This function must not be called for the root memory cgroup. > + * If memcg is the root memory cgroup, this function will iterate only > + * over tasks belonging directly to the root memory cgroup. > */ > int mem_cgroup_scan_tasks(struct mem_cgroup *memcg, > int (*fn)(struct task_struct *, void *), void *arg) > @@ -925,8 +926,6 @@ int mem_cgroup_scan_tasks(struct mem_cgroup *memcg, > struct mem_cgroup *iter; > int ret = 0; > > - BUG_ON(memcg == root_mem_cgroup); > - > for_each_mem_cgroup_tree(iter, memcg) { > struct css_task_iter it; > struct task_struct *task; > @@ -935,7 +934,7 @@ int mem_cgroup_scan_tasks(struct mem_cgroup *memcg, > while (!ret && (task = css_task_iter_next(&it))) > ret = fn(task, arg); > css_task_iter_end(&it); > - if (ret) { > + if (ret || memcg == root_mem_cgroup) { > mem_cgroup_iter_break(memcg, iter); > break; > } I think this is fine, but a little strange to start an iteration that never loops :) No objection to the patch but it could also be extracted into a new mem_cgroup_scan_tasks() which actually scans the tasks in that mem cgroup and then a wrapper that does the iteration that calls into it, say, mem_cgroup_scan_tasks_tree().