On Thu, Jun 18, 2020 at 10:20:05PM -0400, J. Bruce Fields wrote: > On Fri, Jun 19, 2020 at 08:39:48AM +1000, Dave Chinner wrote: > > On Wed, Jun 17, 2020 at 11:45:35PM -0400, Masayoshi Mizuma wrote: > > > Thank you for pointed it out. > > > How about following change? I believe it works both xfs and btrfs... > > > > > > diff --git a/fs/super.c b/fs/super.c > > > index b0a511bef4a0..42fc6334d384 100644 > > > --- a/fs/super.c > > > +++ b/fs/super.c > > > @@ -973,6 +973,9 @@ int reconfigure_super(struct fs_context *fc) > > > } > > > } > > > > > > + if (sb->s_flags & SB_I_VERSION) > > > + fc->sb_flags |= MS_I_VERSION; > > > + > > > WRITE_ONCE(sb->s_flags, ((sb->s_flags & ~fc->sb_flags_mask) | > > > (fc->sb_flags & fc->sb_flags_mask))); > > > /* Needs to be ordered wrt mnt_is_readonly() */ > > > > This will prevent SB_I_VERSION from being turned off at all. That > > will break existing filesystems that allow SB_I_VERSION to be turned > > off on remount, such as ext4. > > > > The manipulations here need to be in the filesystem specific code; > > we screwed this one up so badly there is no "one size fits all" > > behaviour that we can implement in the generic code... > > My memory was that after Jeff Layton's i_version patches, there wasn't > really a significant performance hit any more, so the ability to turn it > off is no longer useful. Yes, I completely agree with you here. However, with some filesystems allowing it to be turned off, we can't just wave our hands and force enable the option. Those filesystems - if the maintainers chose to always enable iversion - will have to go through a mount option deprecation period before permanently enabling it. > But looking back through Jeff's postings, I don't see him claiming that; > e.g. in: > > https://lore.kernel.org/lkml/20171222120556.7435-1-jlayton@xxxxxxxxxx/ > https://lore.kernel.org/linux-nfs/20180109141059.25929-1-jlayton@xxxxxxxxxx/ > https://lore.kernel.org/linux-nfs/1517228795.5965.24.camel@xxxxxxxxxx/ > > he reports comparing old iversion behavior to new iversion behavior, but > not new iversion behavior to new noiversion behavior. Yeah, it's had to compare noiversion behaviour on filesystems where it was understood that it couldn't actually be turned off. And, realistically, the comaprison to noiversion wasn't really relevant to the problem Jeff's patchset was addressing... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
