From: Dave Chinner <david@fromorbit.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>,
"Darrick J. Wong" <djwong@kernel.org>,
Hugh Dickins <hughd@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Chuck Lever <chuck.lever@oracle.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-xfs@vger.kernel.org, linux-mm@kvack.org,
linux-nfs@vger.kernel.org
Subject: Re: [RFC PATCH 0/3][RESEND] fs: opportunistic high-res file timestamps
Date: Wed, 12 Apr 2023 09:13:45 +1000 [thread overview]
Message-ID: <20230411231345.GB3223426@dread.disaster.area> (raw)
In-Reply-To: <20230411143702.64495-1-jlayton@kernel.org>
On Tue, Apr 11, 2023 at 10:36:59AM -0400, Jeff Layton wrote:
> (Apologies for the resend, but I didn't send this with a wide enough
> distribution list originally).
>
> A few weeks ago, during one of the discussions around i_version, Dave
> Chinner wrote this:
>
> "You've missed the part where I suggested lifting the "nfsd sampled
> i_version" state into an inode state flag rather than hiding it in
> the i_version field. At that point, we could optimise away the
> secondary ctime updates just like you are proposing we do with the
> i_version updates. Further, we could also use that state it to
> decide whether we need to use high resolution timestamps when
> recording ctime updates - if the nfsd has not sampled the
> ctime/i_version, we don't need high res timestamps to be recorded
> for ctime...."
>
> While I don't think we can practically optimize away ctime updates
> like we do with i_version, I do like the idea of using this scheme to
> indicate when we need to use a high-res timestamp.
>
> This patchset is a first stab at a scheme to do this. It declares a new
> i_state flag for this purpose and adds two new vfs-layer functions to
> implement conditional high-res timestamp fetching. It then converts both
> tmpfs and xfs to use it.
>
> This seems to behave fine under xfstests, but I haven't yet done
> any performance testing with it. I wouldn't expect it to create huge
> regressions though since we're only grabbing high res timestamps after
> each query.
>
> I like this scheme because we can potentially convert any filesystem to
> use it. No special storage requirements like with i_version field. I
> think it'd potentially improve NFS cache coherency with a whole swath of
> exportable filesystems, and helps out NFSv3 too.
>
> This is really just a proof-of-concept. There are a number of things we
> could change:
>
> 1/ We could use the top bit in the tv_sec field as the flag. That'd give
> us different flags for ctime and mtime. We also wouldn't need to use
> a spinlock.
>
> 2/ We could probably optimize away the high-res timestamp fetch in more
> cases. Basically, always do a coarse-grained ts fetch and only fetch
> the high-res ts when the QUERIED flag is set and the existing time
> hasn't changed.
>
> If this approach looks reasonable, I'll plan to start working on
> converting more filesystems.
Seems reasonable to me. In terms of testing, I suspect the main
impact is going to be the additionaly overhead of taking a spinlock
in normal stat calls. In which case, testing common tools like giti
would be useful. e.g. `git status` runs about 170k stat calls on a
typical kernel tree. If anything is going to be noticed by users
that actually care, it'll be workloads like this...
If we manage to elide the spinlock altogether, then I don't think
we're going to be able to measure any sort perf difference on modern
hardware short of high end NFS benchmarks that drive servers to
their CPU usage limits....
> One thing I'm not clear on is how widely available high res timestamps
> are. Is this something we need to gate on particular CONFIG_* options?
Don't think so - the kernel should always provide the highest
resoultion it can through the get_time interfaces - the _coarse
variants simple return what was read from the high res timer at the
last scheduler tick, hence avoiding the hardware timer overhead when
high res timer resolution is not needed.....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2023-04-11 23:13 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-11 14:36 Jeff Layton
2023-04-11 14:37 ` [RFC PATCH 1/3][RESEND] fs: add infrastructure for opportunistic high-res ctime/mtime updates Jeff Layton
2023-04-11 14:42 ` Darrick J. Wong
2023-04-11 14:54 ` Jeff Layton
2023-04-11 15:07 ` Christian Brauner
2023-04-11 16:04 ` Jeff Layton
2023-04-21 10:23 ` Jan Kara
2023-04-11 14:37 ` [RFC PATCH 2/3][RESEND] shmem: mark for high-res timestamps on next update after getattr Jeff Layton
2023-04-24 7:20 ` kernel test robot
2023-04-11 14:37 ` [RFC PATCH 3/3][RESEND] xfs: mark the inode for high-res timestamp update in getattr Jeff Layton
2023-04-11 14:54 ` Darrick J. Wong
2023-04-11 15:15 ` Christian Brauner
2023-04-11 16:05 ` Jeff Layton
2023-04-11 15:58 ` Jeff Layton
2023-04-21 2:04 ` kernel test robot
2023-04-11 23:13 ` Dave Chinner [this message]
2023-04-15 11:35 ` [RFC PATCH 0/3][RESEND] fs: opportunistic high-res file timestamps Amir Goldstein
2023-04-15 12:13 ` Jeff Layton
2023-04-15 16:19 ` [RFC PATCH 0/3][RESEND] " Chuck Lever III
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230411231345.GB3223426@dread.disaster.area \
--to=david@fromorbit.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=djwong@kernel.org \
--cc=hughd@google.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox