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
