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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CC69C4360C for ; Tue, 8 Oct 2019 04:06:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0286820867 for ; Tue, 8 Oct 2019 04:06:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0286820867 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=fromorbit.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6CBFC8E0005; Tue, 8 Oct 2019 00:06:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 67C9E8E0003; Tue, 8 Oct 2019 00:06:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 56B1C8E0005; Tue, 8 Oct 2019 00:06:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0097.hostedemail.com [216.40.44.97]) by kanga.kvack.org (Postfix) with ESMTP id 35DE58E0003 for ; Tue, 8 Oct 2019 00:06:36 -0400 (EDT) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id B55CC181AC9BA for ; Tue, 8 Oct 2019 04:06:35 +0000 (UTC) X-FDA: 76019280750.04.steel45_5683d85b19f1d X-HE-Tag: steel45_5683d85b19f1d X-Filterd-Recvd-Size: 5809 Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by imf07.hostedemail.com (Postfix) with ESMTP for ; Tue, 8 Oct 2019 04:06:34 +0000 (UTC) Received: from dread.disaster.area (pa49-181-226-196.pa.nsw.optusnet.com.au [49.181.226.196]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 90A4543E409; Tue, 8 Oct 2019 15:06:31 +1100 (AEDT) Received: from dave by dread.disaster.area with local (Exim 4.92.2) (envelope-from ) id 1iHglb-00045v-1P; Tue, 08 Oct 2019 15:06:31 +1100 Date: Tue, 8 Oct 2019 15:06:31 +1100 From: Dave Chinner To: Roman Gushchin Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, tj@kernel.org, Jan Kara Subject: Re: [PATCH] cgroup, blkcg: prevent dirty inodes to pin dying memory cgroups Message-ID: <20191008040630.GA15134@dread.disaster.area> References: <20191004221104.646711-1-guro@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191004221104.646711-1-guro@fb.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=D+Q3ErZj c=1 sm=1 tr=0 a=dRuLqZ1tmBNts2YiI0zFQg==:117 a=dRuLqZ1tmBNts2YiI0zFQg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=XobE76Q3jBoA:10 a=FOH2dFAWAAAA:8 a=7-415B0cAAAA:8 a=tDm895Og1OPoIn_jjw4A:9 a=IdxF-Tp9X7UopkWZ:21 a=VORt4evX2LhOaXV9:21 a=CjuIK1q_8ugA:10 a=i3VuKzQdj-NEYjvDI-p3:22 a=biEYGPWJfzWAr4FL6Ov7:22 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, Oct 04, 2019 at 03:11:04PM -0700, Roman Gushchin wrote: > This is a RFC patch, which is not intended to be merged as is, > but hopefully will start a discussion which can result in a good > solution for the described problem. > > -- > > We've noticed that the number of dying cgroups on our production hosts > tends to grow with the uptime. This time it's caused by the writeback > code. > > An inode which is getting dirty for the first time is associated > with the wb structure (look at __inode_attach_wb()). It can later > be switched to another wb under some conditions (e.g. some other > cgroup is writing a lot of data to the same inode), but generally > stays associated up to the end of life of the inode structure. > > The problem is that the wb structure holds a reference to the original > memory cgroup. So if the inode was dirty once, it has a good chance > to pin down the original memory cgroup. > > An example from the real life: some service runs periodically and > updates rpm packages. Each time in a new memory cgroup. Installed > .so files are heavily used by other cgroups, so corresponding inodes > tend to stay alive for a long. So do pinned memory cgroups. > In production I've seen many hosts with 1-2 thousands of dying > cgroups. > > This is not the first problem with the dying memory cgroups. As > always, the problem is with their relative size: memory cgroups > are large objects, easily 100x-1000x larger that inodes. So keeping > a couple of thousands of dying cgroups in memory without a good reason > (what we easily do with inodes) is quite costly (and is measured > in tens and hundreds of Mb). > > One possible approach to this problem is to switch inodes associated > with dying wbs to the root wb. Switching is a best effort operation > which can fail silently, so unfortunately we can't run once over a > list of associated inodes (even if we'd have such a list). So we > really have to scan all inodes. > > In the proposed patch I schedule a work on each memory cgroup > deletion, which is probably too often. Alternatively, we can do it > periodically under some conditions (e.g. the number of dying memory > cgroups is larger than X). So it's basically a gc run. > > I wonder if there are any better ideas? > > Signed-off-by: Roman Gushchin > --- > fs/fs-writeback.c | 29 +++++++++++++++++++++++++++++ > mm/memcontrol.c | 5 +++++ > 2 files changed, 34 insertions(+) > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > index 542b02d170f8..4bbc9a200b2c 100644 > --- a/fs/fs-writeback.c > +++ b/fs/fs-writeback.c > @@ -545,6 +545,35 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id) > up_read(&bdi->wb_switch_rwsem); > } > > +static void reparent_dirty_inodes_one_sb(struct super_block *sb, void *arg) > +{ > + struct inode *inode, *next; > + > + spin_lock(&sb->s_inode_list_lock); > + list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) { > + spin_lock(&inode->i_lock); > + if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) { > + spin_unlock(&inode->i_lock); > + continue; > + } > + > + if (inode->i_wb && wb_dying(inode->i_wb)) { > + spin_unlock(&inode->i_lock); > + inode_switch_wbs(inode, root_mem_cgroup->css.id); > + continue; > + } > + > + spin_unlock(&inode->i_lock); > + } > + spin_unlock(&sb->s_inode_list_lock); No idea what the best solution is, but I think this is fundamentally unworkable. It's not uncommon to have a hundred million cached inodes these days, often on a single filesystem. Anything that requires a brute-force system wide inode scan, especially without conditional reschedule points, is largely a non-starter. Also, inode_switch_wbs() is not guaranteed to move the inode to the destination wb. There can only be WB_FRN_MAX_IN_FLIGHT (1024) switches in flight at once and switches are run via RCU callbacks, so I suspect that using inode_switch_wbs() for bulk re-assignment is going to be a lot more complex than just finding inodes to call inode_switch_wbs() on.... Cheers, Dave. -- Dave Chinner david@fromorbit.com