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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 EC667C2B9F4 for ; Mon, 14 Jun 2021 22:41:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 915666124B for ; Mon, 14 Jun 2021 22:41:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 915666124B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1489F6B006C; Mon, 14 Jun 2021 18:41:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0D5946B006E; Mon, 14 Jun 2021 18:41:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8DC36B0070; Mon, 14 Jun 2021 18:41:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0042.hostedemail.com [216.40.44.42]) by kanga.kvack.org (Postfix) with ESMTP id B2B6E6B006C for ; Mon, 14 Jun 2021 18:41:52 -0400 (EDT) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 5107C181AC9CB for ; Mon, 14 Jun 2021 22:41:52 +0000 (UTC) X-FDA: 78253803264.22.6359A23 Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf25.hostedemail.com (Postfix) with ESMTP id 3D5246000147 for ; Mon, 14 Jun 2021 22:41:43 +0000 (UTC) Received: by mail-qv1-f44.google.com with SMTP id g12so21302287qvx.12 for ; Mon, 14 Jun 2021 15:41:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=+Ln8aa8N4NkXgGmfhKmW+YOk+ZQP4J2F53dGDWutRYI=; b=BTvt9AQQnZeMgoi8kZ+rpU61FQu7oIAu4pBLZ5r/GsD6sENIQpoQzs+ndGR/K6WAKZ qHJImr2H1AtdDSzcc3TZtyfTdpq0bes1JARrugpd/KUJ2tWxjQ1V9vreICAV7fx2X1u6 aRtfU7IuQhAFCv/u//PANv7OV5J2vzkmCgpOZxzIoiaSVZhraKLWOetn+z2Hih5c4hPG dkkrTLKYMAjTsdIy0kLLlJxrp0uro8qMDl6F6G0rt5LnyUDFv2ar8khX9bBW95y8dObH Wl9LkCA9kwiCVTZuNqJ3M0iSrlHQcvNu8MNjaj1IN2RUZ0usAHbZY4QY8Z/n3TO1P0QG vd7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+Ln8aa8N4NkXgGmfhKmW+YOk+ZQP4J2F53dGDWutRYI=; b=p/4B43YWZ/mI5W+iifXaXAXCOPC/e8B4hm9zytYijNS0d0tH7ZLwpBzvll3/H+tD85 YW/8xyfd6WjifALS6AK2+OXh71scMlCLbk5HtI9cbSh+G0NYNArWUZuCNA4MfbyJdVFQ NayzTnnh8QDmUjeC6ILX3zO5qravtMN1UBFcI3BK2t9iHJ2iyfjLl3+YdmEy0M5K2qc+ oL6QOVa2jziti+qPLjDpKBn+Gp0bZ9iUXpfxxvdlaQcSrAKup502tiGZUSvu3RM6svHf cRPvfqDAwKlc4j+/n+cjEvz2Rnpk63TLuDpeY+mynOnp4Qz+44f/XZFmsqg2ixtPzODy NNgA== X-Gm-Message-State: AOAM530tmWIRkjv9clM6wlS1aeqKmotoqMqGXy0BYXdbKt+ti7iGHucL iRpJGVNIFS3va+/a6KzhMMgx+g== X-Google-Smtp-Source: ABdhPJyWQrrXQUjJrZcR1DdcwITE7ckgVpPOl79Nc9WscxBEYiYeedtKgqsg2s8LjpIEFtocJ72oSQ== X-Received: by 2002:ad4:5042:: with SMTP id m2mr315121qvq.29.1623710510887; Mon, 14 Jun 2021 15:41:50 -0700 (PDT) Received: from localhost (70.44.39.90.res-cmts.bus.ptd.net. [70.44.39.90]) by smtp.gmail.com with ESMTPSA id g1sm1518745qkd.125.2021.06.14.15.41.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Jun 2021 15:41:50 -0700 (PDT) Date: Mon, 14 Jun 2021 18:41:49 -0400 From: Johannes Weiner To: Andrew Morton Cc: Roman Gushchin , Tejun Heo , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Dave Chinner Subject: Re: [PATCH 4/4] vfs: keep inodes with page cache off the inode shrinker LRU Message-ID: References: <20210614211904.14420-1-hannes@cmpxchg.org> <20210614211904.14420-4-hannes@cmpxchg.org> <20210614145912.feb751df928f38476048ec15@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210614145912.feb751df928f38476048ec15@linux-foundation.org> Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=cmpxchg-org.20150623.gappssmtp.com header.s=20150623 header.b=BTvt9AQQ; spf=pass (imf25.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.44 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 3D5246000147 X-Stat-Signature: ygjhduy7zuzrrg6f7p5a14brtpxqaare X-HE-Tag: 1623710503-969868 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 Mon, Jun 14, 2021 at 02:59:12PM -0700, Andrew Morton wrote: > On Mon, 14 Jun 2021 17:19:04 -0400 Johannes Weiner wrote: > > > Historically (pre-2.5), the inode shrinker used to reclaim only empty > > inodes and skip over those that still contained page cache. This > > caused problems on highmem hosts: struct inode could put fill lowmem > > zones before the cache was getting reclaimed in the highmem zones. > > > > To address this, the inode shrinker started to strip page cache to > > facilitate reclaiming lowmem. However, this comes with its own set of > > problems: the shrinkers may drop actively used page cache just because > > the inodes are not currently open or dirty - think working with a > > large git tree. It further doesn't respect cgroup memory protection > > settings and can cause priority inversions between containers. > > > > Nowadays, the page cache also holds non-resident info for evicted > > cache pages in order to detect refaults. We've come to rely heavily on > > this data inside reclaim for protecting the cache workingset and > > driving swap behavior. We also use it to quantify and report workload > > health through psi. The latter in turn is used for fleet health > > monitoring, as well as driving automated memory sizing of workloads > > and containers, proactive reclaim and memory offloading schemes. > > > > The consequences of dropping page cache prematurely is that we're > > seeing subtle and not-so-subtle failures in all of the above-mentioned > > scenarios, with the workload generally entering unexpected thrashing > > states while losing the ability to reliably detect it. > > > > To fix this on non-highmem systems at least, going back to rotating > > inodes on the LRU isn't feasible. We've tried (commit a76cf1a474d7 > > ("mm: don't reclaim inodes with many attached pages")) and failed > > (commit 69056ee6a8a3 ("Revert "mm: don't reclaim inodes with many > > attached pages"")). The issue is mostly that shrinker pools attract > > pressure based on their size, and when objects get skipped the > > shrinkers remember this as deferred reclaim work. This accumulates > > excessive pressure on the remaining inodes, and we can quickly eat > > into heavily used ones, or dirty ones that require IO to reclaim, when > > there potentially is plenty of cold, clean cache around still. > > > > Instead, this patch keeps populated inodes off the inode LRU in the > > first place - just like an open file or dirty state would. An > > otherwise clean and unused inode then gets queued when the last cache > > entry disappears. This solves the problem without reintroducing the > > reclaim issues, and generally is a bit more scalable than having to > > wade through potentially hundreds of thousands of busy inodes. > > > > Locking is a bit tricky because the locks protecting the inode state > > (i_lock) and the inode LRU (lru_list.lock) don't nest inside the > > irq-safe page cache lock (i_pages.xa_lock). Page cache deletions are > > serialized through i_lock, taken before the i_pages lock, to make sure > > depopulated inodes are queued reliably. Additions may race with > > deletions, but we'll check again in the shrinker. If additions race > > with the shrinker itself, we're protected by the i_lock: if > > find_inode() or iput() win, the shrinker will bail on the elevated > > i_count or I_REFERENCED; if the shrinker wins and goes ahead with the > > inode, it will set I_FREEING and inhibit further igets(), which will > > cause the other side to create a new instance of the inode instead. > > > > And what hitherto unexpected problems will this one cause, sigh. Yeah, I wish we could have stuck with simple rotations, but I can see how they can become problematic - even though we haven't been able to reproduce the issue which led to the revert in the FB fleet. > How exhaustively has this approach been tested? This specific patch I've put through various inode / cache loads on my local test rig, drgn-inspected the sb inode lists to check for leaked inodes etc. - they've held up. We've had the previous version (which was more complicated) running on the entire Facebook fleet for a while. I did switch us back to simple rotations in a recent rebase, though, because the one-liner is easier to carry out of tree and (luckily) isn't causing problems for us. It would be good to have a fix upstream that works for everybody, though. The above-mentioned containerization and pressure detection failures aren't theoretical, we can't run upstream in our fleet. We're also trying to publish containerization software that showcases how the different cgroup controllers play together, how automated container sizing works etc, but many of those scenarios are broken in the upstream kernel. A kernel patch dependency is a painful one for a project like this. Tejun can probably provide more context on this.