linux-kernel - Re: [RFC PATCH 1/3][RESEND] fs: add infrastructure for opportunistic high-res ctime/mtime updates

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]

Date:   Fri, 21 Apr 2023 12:23:53 +0200
From:   Jan Kara <jack@...e.cz>
To:     Jeff Layton <jlayton@...nel.org>
Cc:     Christian Brauner <brauner@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        "Darrick J. Wong" <djwong@...nel.org>,
        Hugh Dickins <hughd@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dave Chinner <david@...morbit.com>,
        Chuck Lever <chuck.lever@...cle.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-mm@...ck.org,
        linux-nfs@...r.kernel.org
Subject: Re: [RFC PATCH 1/3][RESEND] fs: add infrastructure for opportunistic
 high-res ctime/mtime updates

On Tue 11-04-23 12:04:36, Jeff Layton wrote:
> On Tue, 2023-04-11 at 17:07 +0200, Christian Brauner wrote:
> > On Tue, Apr 11, 2023 at 10:37:00AM -0400, Jeff Layton wrote:
> > There's some performance concerns here. Calling
> > stat() is super common and it would potentially make the next iop more
> > expensive. Recursively changing ownership in the container use-case come
> > to mind which are already expensive.
> 
> stat() is common, but not generally as common as write calls are. I
> expect that we'll get somewhat similar results tochanged i_version over
> to use a similar QUERIED flag.
> 
> The i_version field was originally very expensive and required metadata
> updates on every write. After making that change, we got the same
> performance back in most tests that we got without the i_version field
> being enabled at all. Basically, this just means we'll end up logging an
> extra journal transaction on some writes that follow a stat() call,
> which turns out to be line noise for most workloads.
> 
> I do agree that performance is a concern here though. We'll need to
> benchmark this somehow.

So for stat-intensive read-only workloads the additional inode_lock locking
during stat may be noticeable. I suppose a stress test stating the same
file in a loop from all CPUs the machine has will certainly notice :) But
that's just an unrealistic worst case.

We could check whether the QUERIED flag is already set and if yes, skip the
locking. That should fix the read-only workload case. We just have to think
whether there would not be some unpleasant races created.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR