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

[Jan Kara]

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@xxxxxxxx>
SUSE Labs, CR