From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6EAFCC433F5 for ; Sat, 23 Apr 2022 00:36:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D319F6B0073; Fri, 22 Apr 2022 20:36:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CDFAE6B0074; Fri, 22 Apr 2022 20:36:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B816A6B0075; Fri, 22 Apr 2022 20:36:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.27]) by kanga.kvack.org (Postfix) with ESMTP id A962B6B0073 for ; Fri, 22 Apr 2022 20:36:08 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 733DB2430B for ; Sat, 23 Apr 2022 00:36:08 +0000 (UTC) X-FDA: 79386276816.07.CA09956 Received: from r3-21.sinamail.sina.com.cn (r3-21.sinamail.sina.com.cn [202.108.3.21]) by imf22.hostedemail.com (Postfix) with SMTP id 1C7C5C0006 for ; Sat, 23 Apr 2022 00:36:05 +0000 (UTC) Received: from unknown (HELO localhost.localdomain)([114.249.57.134]) by sina.com (172.16.97.32) with ESMTP id 626349DE0002AE4B; Sat, 23 Apr 2022 08:35:44 +0800 (CST) X-Sender: hdanton@sina.com X-Auth-ID: hdanton@sina.com X-SMAIL-MID: 242954628803 From: Hillf Danton To: Roman Gushchin Cc: Andrew Morton , linux-mm@kvack.org, Dave Chinner , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/7] mm: introduce memcg interfaces for shrinker debugfs Date: Sat, 23 Apr 2022 08:35:52 +0800 Message-Id: <20220423003552.2914-1-hdanton@sina.com> In-Reply-To: <20220422202644.799732-4-roman.gushchin@linux.dev> References: <20220422202644.799732-1-roman.gushchin@linux.dev> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Stat-Signature: cwnnhwo961e7tgejp8bu6fu7xiorsihz Authentication-Results: imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of hdanton@sina.com designates 202.108.3.21 as permitted sender) smtp.mailfrom=hdanton@sina.com; dmarc=none X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 1C7C5C0006 X-HE-Tag: 1650674165-372301 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, 22 Apr 2022 13:26:40 -0700 Roman Gushchin wrote: > This commit introduces "count_memcg" and "scan_memcg" interfaces > for memcg-aware shrinkers. > > Count_memcg using the following format: > > > ... > > Memory cgroups with 0 associated objects are skipped. > > Signed-off-by: Roman Gushchin > --- > mm/shrinker_debug.c | 186 +++++++++++++++++++++++++++++++++----------- > 1 file changed, 139 insertions(+), 47 deletions(-) > > diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c > index 4df7382a0737..002d44d6ad56 100644 > --- a/mm/shrinker_debug.c > +++ b/mm/shrinker_debug.c > @@ -1,8 +1,10 @@ > // SPDX-License-Identifier: GPL-2.0 > #include > +#include > #include > #include > #include > +#include > > /* defined in vmscan.c */ > extern struct rw_semaphore shrinker_rwsem; > @@ -11,25 +13,25 @@ extern struct list_head shrinker_list; > static DEFINE_IDA(shrinker_debugfs_ida); > static struct dentry *shrinker_debugfs_root; > > -static int shrinker_debugfs_count_show(struct seq_file *m, void *v) > +static unsigned long shrinker_count_objects(struct shrinker *shrinker, > + struct mem_cgroup *memcg, > + unsigned long *count_per_node) > { > - struct shrinker *shrinker = (struct shrinker *)m->private; > unsigned long nr, total = 0; > - int ret, nid; > - > - ret = down_read_killable(&shrinker_rwsem); > - if (ret) > - return ret; > + int nid; > > for_each_node(nid) { > struct shrink_control sc = { > .gfp_mask = GFP_KERNEL, > .nid = nid, > + .memcg = memcg, > }; > > nr = shrinker->count_objects(shrinker, &sc); > if (nr == SHRINK_EMPTY) > nr = 0; > + if (count_per_node) > + count_per_node[nid] = nr; > total += nr; > > if (!(shrinker->flags & SHRINKER_NUMA_AWARE)) > @@ -37,32 +39,17 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v) > > cond_resched(); Nit, add a line in response to signal before schedule, given the killable above. > } > - up_read(&shrinker_rwsem); > - > - seq_printf(m, "%lu\n", total); > > - return ret; > + return total; > } > -DEFINE_SHOW_ATTRIBUTE(shrinker_debugfs_count); > > -static ssize_t shrinker_debugfs_scan_write(struct file *file, > - const char __user *buf, > - size_t size, loff_t *pos) > +static int shrinker_scan_objects(struct shrinker *shrinker, > + struct mem_cgroup *memcg, > + unsigned long nr_to_scan) > { > - struct shrinker *shrinker = (struct shrinker *)file->private_data; > - unsigned long nr, total = 0, nr_to_scan; > - unsigned long *count_per_node = NULL; > - int nid; > - char kbuf[24]; > - int read_len = size < (sizeof(kbuf) - 1) ? size : (sizeof(kbuf) - 1); > - ssize_t ret; > - > - if (copy_from_user(kbuf, buf, read_len)) > - return -EFAULT; > - kbuf[read_len] = '\0'; > - > - if (kstrtoul(kbuf, 10, &nr_to_scan)) > - return -EINVAL; > + unsigned long *count_per_node; > + unsigned long total, nr; > + int ret, nid; > > ret = down_read_killable(&shrinker_rwsem); > if (ret) > @@ -80,20 +67,7 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, > goto out; > } > > - for_each_node(nid) { > - struct shrink_control sc = { > - .gfp_mask = GFP_KERNEL, > - .nid = nid, > - }; > - > - nr = shrinker->count_objects(shrinker, &sc); > - if (nr == SHRINK_EMPTY) > - nr = 0; > - count_per_node[nid] = nr; > - total += nr; > - > - cond_resched(); > - } > + total = shrinker_count_objects(shrinker, memcg, count_per_node); > } > > for_each_node(nid) { > @@ -102,13 +76,13 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, > .nid = nid, > }; > > - if (shrinker->flags & SHRINKER_NUMA_AWARE) { > + if (count_per_node) { > sc.nr_to_scan = nr_to_scan * count_per_node[nid] / > (total ? total : 1); > sc.nr_scanned = sc.nr_to_scan; > } else { > sc.nr_to_scan = nr_to_scan; > - sc.nr_scanned = sc.nr_to_scan; > + sc.nr_scanned = nr_to_scan; > } > > nr = shrinker->scan_objects(shrinker, &sc); > @@ -119,15 +93,51 @@ static ssize_t shrinker_debugfs_scan_write(struct file *file, > break; > > cond_resched(); > - > } > - ret = size; > out: > up_read(&shrinker_rwsem); > kfree(count_per_node); > return ret; > } > > +static int shrinker_debugfs_count_show(struct seq_file *m, void *v) > +{ > + struct shrinker *shrinker = (struct shrinker *)m->private; > + int ret; > + > + ret = down_read_killable(&shrinker_rwsem); > + if (!ret) { > + unsigned long total = shrinker_count_objects(shrinker, NULL, NULL); > + > + up_read(&shrinker_rwsem); > + seq_printf(m, "%lu\n", total); > + } > + return ret; > +} > +DEFINE_SHOW_ATTRIBUTE(shrinker_debugfs_count); > + > +static ssize_t shrinker_debugfs_scan_write(struct file *file, > + const char __user *buf, > + size_t size, loff_t *pos) > +{ > + struct shrinker *shrinker = (struct shrinker *)file->private_data; > + unsigned long nr_to_scan; > + char kbuf[24]; > + int read_len = size < (sizeof(kbuf) - 1) ? size : (sizeof(kbuf) - 1); > + ssize_t ret; > + > + if (copy_from_user(kbuf, buf, read_len)) > + return -EFAULT; > + kbuf[read_len] = '\0'; > + > + if (kstrtoul(kbuf, 10, &nr_to_scan)) > + return -EINVAL; > + > + ret = shrinker_scan_objects(shrinker, NULL, nr_to_scan); > + > + return ret ? ret : size; > +} > + > static int shrinker_debugfs_scan_open(struct inode *inode, struct file *file) > { > file->private_data = inode->i_private; > @@ -140,6 +150,78 @@ static const struct file_operations shrinker_debugfs_scan_fops = { > .write = shrinker_debugfs_scan_write, > }; > > +#ifdef CONFIG_MEMCG > +static int shrinker_debugfs_count_memcg_show(struct seq_file *m, void *v) > +{ > + struct shrinker *shrinker = (struct shrinker *)m->private; > + struct mem_cgroup *memcg; > + unsigned long total; > + int ret; > + > + ret = down_read_killable(&shrinker_rwsem); > + if (ret) > + return ret; > + rcu_read_lock(); A minute ... things like cond_resched() or mutex in individual shrinker implementation can ruin your nice work within two seconds. The bigger pain is can we rule them out from coming shrinkers? Hillf > + > + memcg = mem_cgroup_iter(NULL, NULL, NULL); > + do { Nit, take a look at pending signal. > + if (!mem_cgroup_online(memcg)) > + continue; > + > + total = shrinker_count_objects(shrinker, memcg, NULL); > + if (!total) > + continue; > + > + seq_printf(m, "%lu %lu\n", mem_cgroup_ino(memcg), total); > + } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL); > + > + rcu_read_unlock(); > + up_read(&shrinker_rwsem); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(shrinker_debugfs_count_memcg);