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: <aK980KTSlSViOWXW@dread.disaster.area>
Date: Thu, 28 Aug 2025 07:46:56 +1000
From: Dave Chinner <david@...morbit.com>
To: Josef Bacik <josef@...icpanda.com>
Cc: linux-fsdevel@...r.kernel.org, linux-btrfs@...r.kernel.org,
	kernel-team@...com, linux-ext4@...r.kernel.org,
	linux-xfs@...r.kernel.org, brauner@...nel.org,
	viro@...iv.linux.org.uk, amir73il@...il.com
Subject: Re: [PATCH v2 16/54] fs: delete the inode from the LRU list on lookup

On Tue, Aug 26, 2025 at 11:39:16AM -0400, Josef Bacik wrote:
> When we move to holding a full reference on the inode when it is on an
> LRU list we need to have a mechanism to re-run the LRU add logic. The
> use case for this is btrfs's snapshot delete, we will lookup all the
> inodes and try to drop them, but if they're on the LRU we will not call
> ->drop_inode() because their refcount will be elevated, so we won't know
> that we need to drop the inode.
> 
> Fix this by simply removing the inode from it's respective LRU list when
> we grab a reference to it in a way that we have active users.  This will
> ensure that the logic to add the inode to the LRU or drop the inode will
> be run on the final iput from the user.
> 
> Signed-off-by: Josef Bacik <josef@...icpanda.com>

Have you benchmarked this for scalability?

The whole point of lazy LRU removal was to remove LRU lock
contention from the hot lookup path. I suspect that putting the LRU
locks back inside the lookup path is going to cause performance
regressions...

FWIW, why do we even need the inode LRU anymore?

We certainly don't need it anymore to keep the working set in memory
because that's what the dentry cache LRU does (i.e. by pinning a
reference to the inode whilst the dentry is active).

And with the introduction of the cached inode list, we don't need
the inode LRU to track  unreferenced dirty inodes around whilst
they hang out on writeback lists. The inodes on the writeback lists
are now referenced and tracked on the cached inode list, so they
don't need special hooks in the mm/ code to handle the special
transition from "unreferenced writeback" to "unreferenced LRU"
anymore, they can just be dropped from the cached inode list....

So rather than jumping through hoops to maintain an LRU we likely
don't actually need and is likely to re-introduce old scalability
issues, why not remove it completely?

-Dave.
-- 
Dave Chinner
david@...morbit.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ