From: Jeff Layton <jlayton@kernel.org>
To: Lukas Czerner <lczerner@redhat.com>
Cc: tytso@mit.edu, adilger.kernel@dilger.ca,
linux-ext4@vger.kernel.org,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Benjamin Coddington <bcodding@redhat.com>
Subject: Re: should we make "-o iversion" the default on ext4 ?
Date: Wed, 20 Jul 2022 10:38:31 -0400 [thread overview]
Message-ID: <ad7218a41fa8ac26911a9ccb79c87609d4279fea.camel@kernel.org> (raw)
In-Reply-To: <20220720141546.46l2d7bxwukjhtl7@fedora>
On Wed, 2022-07-20 at 16:15 +0200, Lukas Czerner wrote:
> On Tue, Jul 19, 2022 at 09:51:33AM -0400, Jeff Layton wrote:
> > Back in 2018, I did a patchset [1] to rework the inode->i_version
> > counter handling to be much less expensive, particularly when no-one is
> > querying for it.
> >
> > Testing at the time showed that the cost of enabling i_version on ext4
> > was close to 0 when nothing is querying it, but I stopped short of
> > trying to make it the default at the time (mostly out of an abundance of
> > caution). Since then, we still see a steady stream of cache-coherency
> > problems with NFSv4 on ext4 when this option is disabled (e.g. [2]).
> >
> > Is it time to go ahead and make this option the default on ext4? I don't
> > see a real downside to doing so, though I'm unclear on how we should
> > approach this. Currently the option is twiddled using MS_I_VERSION flag,
> > and it's unclear to me how we can reverse the sense of such a flag.
> >
> > Thoughts?
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4b7fd7d34de5765dece2dd08060d2e1f7be3b39
> > [2]: https://bugzilla.redhat.com/show_bug.cgi?id=2107587
>
> Hi,
>
> I don't have the results myself yet, but a quick look at how it is done
> suggests that indeed the impact should be low. But not zero, at least
> every time the inode is loaded from disk it is scheduled for i_version
> update on the next attempted increment. Could that have an effect on
> some particular common workload you can think of?
>
Yeah, it's not zero, but hopefully any performance hit would end up
amortized over the long term use of the inode. In the common situation
where the i_version flag hasn't been queried, this just ends up being an
extra atomic fetch to look at i_version and detect that.
> How would we approach making iversion a default? libmount is passing
> this option to the kernel as just a MS_I_VERSION flag that is set when
> -o iversion is used and left empty when the -o noiversion is used. This
> means that while we could make it a default in ext4, we don't have any
> way of knowing whether the user asked for -o noiversion. So that's not
> really an option.
>
> Updating the mke2fs/tune2fs to allow setting iversion as a default mount
> option I think has the same problem.
>
> So the only way I can see ATM would be to introduce another mountflag
> for libmount to indicate -o noiversion. This way we can make iversion a
> default on ext4 without loosing the information about user provided -o
> noiversion option.
>
> Is there a different way I am not seeing?
>
Right, implementing this is the difficult bit actually since this uses a
MS_* flag. If we do make this the default, we'd definitely want to
continue allowing "-o noiversion" to disable it.
Could we just reverse the default in libmount? It might cause this to
suddenly be enabled in some deployments, but in most cases, people
wouldn't even notice and they could still specify -o noiversion to turn
it off.
Another idea would be to introduce new mount options for this, but
that's kind of nasty from a UI standpoint.
>
> If we can do reasonably extensive testing that will indeed show
> negligible impact when nothing is querying i_version, then I would be in
> favor of the change.
>
Excellent! I think that would be best if we can get away with it. A lot
of people are currently running ext4-backed nfs servers and aren't using
that mount option.
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2022-07-20 14:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-19 13:51 should we make "-o iversion" the default on ext4 ? Jeff Layton
2022-07-20 14:15 ` Lukas Czerner
2022-07-20 14:38 ` Jeff Layton [this message]
2022-07-20 15:22 ` Lukas Czerner
2022-07-20 16:42 ` Jeff Layton
2022-07-21 14:06 ` Lukas Czerner
2022-07-21 17:03 ` Jeff Layton
2022-07-20 15:56 ` Benjamin Coddington
2022-07-20 16:06 ` Christoph Hellwig
2022-07-20 16:15 ` Jeff Layton
2022-07-20 16:29 ` Benjamin Coddington
2022-07-20 16:46 ` Jeff Layton
2022-07-21 22:32 ` Dave Chinner
2022-07-25 16:22 ` Jeff Layton
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=ad7218a41fa8ac26911a9ccb79c87609d4279fea.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=adilger.kernel@dilger.ca \
--cc=bcodding@redhat.com \
--cc=lczerner@redhat.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=tytso@mit.edu \
/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;
as well as URLs for NNTP newsgroup(s).