linux-kernel - Re: [PATCH] vfs: keep inodes with page cache off the inode shrinker LRU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200513141519.061f8fca4788cd02b4d7068f@linux-foundation.org>
Date:   Wed, 13 May 2020 14:15:19 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Dave Chinner <david@...morbit.com>,
        Yafang Shao <laoar.shao@...il.com>,
        Michal Hocko <mhocko@...e.com>, Roman Gushchin <guro@...com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>, kernel-team@...com
Subject: Re: [PATCH] vfs: keep inodes with page cache off the inode shrinker
 LRU

On Tue, 12 May 2020 17:29:36 -0400 Johannes Weiner <hannes@...xchg.org> wrote:

> 
> ...
>
> Solution
> 
> This patch fixes the aging inversion described above on
> !CONFIG_HIGHMEM systems, without reintroducing the problems associated
> with excessive shrinker LRU rotations, by keeping populated inodes off
> the shrinker LRUs entirely.
> 
> Currently, inodes are kept off the shrinker LRU as long as they have
> an elevated i_count, indicating an active user. Unfortunately, the
> page cache cannot simply hold an i_count reference, because unlink()
> *should* result in the inode being dropped and its cache invalidated.
> 
> Instead, this patch makes iput_final() consult the state of the page
> cache and punt the LRU linking to the VM if the inode is still
> populated; the VM in turn checks the inode state when it depopulates
> the page cache, and adds the inode to the LRU if necessary.
> 
> This is not unlike what we do for dirty inodes, which are moved off
> the LRU permanently until writeback completion puts them back on (iff
> still unused). We can reuse the same code -- inode_add_lru() - here.
> 
> This is also not unlike page reclaim, where the lower VM layer has to
> negotiate state with the higher VFS layer. Follow existing precedence
> and handle the inversion as much as possible on the VM side:
> 
> - introduce an I_PAGES flag that the VM maintains under the i_lock, so
>   that any inode code holding that lock can check the page cache state
>   without having to lock and inspect the struct address_space

Maintaining the same info in two places is a hassle.  Is this
optimization worthwhile?

> - introduce inode_pages_set() and inode_pages_clear() to maintain the
>   inode LRU state from the VM side, then update all cache mutators to
>   use them when populating the first cache entry or clearing the last
> 
> With this, the concept of "inodesteal" - where the inode shrinker
> drops page cache - is relegated to CONFIG_HIGHMEM systems only. The VM
> is in charge of the cache, the shrinker in charge of struct inode.

How tested is this on highmem machines?

> Footnotes
> 
> - For debuggability, add vmstat counters that track the number of
>   times a new cache entry pulls a previously unused inode off the LRU
>   (pginoderescue), as well as how many times existing cache deferred
>   an LRU addition. Keep the pginodesteal/kswapd_inodesteal counters
>   for backwards compatibility, but they'll just show 0 now.
> 
> - Fix /proc/sys/vm/drop_caches to drop shadow entries from the page
>   cache. Not doing so has always been a bit strange, but since most
>   people drop cache and metadata cache together, the inode shrinker
>   would have taken care of them before - no more, so do it VM-side.
> 
> ...
>
>  14 files changed, 208 insertions(+), 34 deletions(-)

Patch is surprisingly large.