mbox series

[GIT,PULL] vfs: improve DAX behavior for 5.8, part 3

Message ID 20200611024248.GG11245@magnolia
State Not Applicable
Headers show
Series [GIT,PULL] vfs: improve DAX behavior for 5.8, part 3 | expand

Pull-request

git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/vfs-5.8-merge-3

Message

Darrick J. Wong June 11, 2020, 2:42 a.m. UTC
Hi Linus,

Please pull this third part of the 5.8 DAX changes.  Now that the xfs
changes have landed, this third piece changes the FS_XFLAG_DAX ioctl
code in xfs to request that the inode be reloaded after the last program
closes the file, if doing so would make a S_DAX change happen.  This
goal here is to make dax access mode switching quicker when possible.

I did a test merge of this branch against upstream this evening and
there weren't any conflicts.  The first five patches in the series were
already in the xfs merge, so it's only the last one that should change
anything.  Please let us know if you have any complaints about pulling
this, since I can rework the branch.

--D

The following changes since commit 2c567af418e3f9380c2051aada58b4e5a4b5c2ad:

  fs: Introduce DCACHE_DONTCACHE (2020-05-13 08:44:35 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/vfs-5.8-merge-3

for you to fetch changes up to e4f9ba20d3b8c2b86ec71f326882e1a3c4e47953:

  fs/xfs: Update xfs_ioctl_setattr_dax_invalidate() (2020-05-29 20:13:20 -0700)

----------------------------------------------------------------
Third part of new DAX code for 5.8:
- Teach XFS to ask the VFS to drop an inode if the administrator changes
  the FS_XFLAG_DAX inode flag such that the S_DAX state would change.
  This can result in files changing access modes without requiring an
  unmount cycle.

----------------------------------------------------------------
Ira Weiny (6):
      fs/xfs: Remove unnecessary initialization of i_rwsem
      fs/xfs: Change XFS_MOUNT_DAX to XFS_MOUNT_DAX_ALWAYS
      fs/xfs: Make DAX mount option a tri-state
      fs/xfs: Create function xfs_inode_should_enable_dax()
      fs/xfs: Combine xfs_diflags_to_linux() and xfs_diflags_to_iflags()
      fs/xfs: Update xfs_ioctl_setattr_dax_invalidate()

 fs/xfs/xfs_icache.c |   4 +-
 fs/xfs/xfs_inode.h  |   1 +
 fs/xfs/xfs_ioctl.c  | 141 ++++++++--------------------------------------------
 fs/xfs/xfs_iops.c   |  70 +++++++++++++++++---------
 fs/xfs/xfs_mount.h  |   4 +-
 fs/xfs/xfs_super.c  |  48 ++++++++++++++++--
 6 files changed, 115 insertions(+), 153 deletions(-)

Comments

Linus Torvalds June 11, 2020, 6 p.m. UTC | #1
On Wed, Jun 10, 2020 at 7:43 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> I did a test merge of this branch against upstream this evening and
> there weren't any conflicts.  The first five patches in the series were
> already in the xfs merge, so it's only the last one that should change
> anything.  Please let us know if you have any complaints about pulling
> this, since I can rework the branch.

I've taken this, but I hate how the patches apparently got duplicated.
It feels like they should have been a cleanly separated branch that
was just pulled into whoever needed them when they were ready, rather
than applied in two different places.

So this is just a note for future work - duplicating the patches like
this can cause annoyances down the line. No merge issues this time
(they often happen when duplicate patches then have other work done on
top of them), but things like "git bisect" now don't have quite as
black-and-white a situation etc etc.,

("git bisect" will still find _one_ of the duplicate commits if it
introduced a problem, so it's usually not a huge deal, but it can
cause the bug to be then repeated if people revert that one, but
nobody ever notices that the other commit that did the same thing is
still around and it gets back-ported to stable or whatever..)

So part of this is just in general about confusing duplicate history,
and part of it is that the duplication can then cause later confusion.

                Linus
pr-tracker-bot@kernel.org June 11, 2020, 6:25 p.m. UTC | #2
The pull request you sent on Wed, 10 Jun 2020 19:42:48 -0700:

> git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/vfs-5.8-merge-3

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7cf035cc8326066a514146065b6ee8fc2c30fc21

Thank you!
Darrick J. Wong June 13, 2020, 5:32 a.m. UTC | #3
On Thu, Jun 11, 2020 at 11:00:43AM -0700, Linus Torvalds wrote:
> On Wed, Jun 10, 2020 at 7:43 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > I did a test merge of this branch against upstream this evening and
> > there weren't any conflicts.  The first five patches in the series were
> > already in the xfs merge, so it's only the last one that should change
> > anything.  Please let us know if you have any complaints about pulling
> > this, since I can rework the branch.
> 
> I've taken this, but I hate how the patches apparently got duplicated.
> It feels like they should have been a cleanly separated branch that
> was just pulled into whoever needed them when they were ready, rather
> than applied in two different places.
> 
> So this is just a note for future work - duplicating the patches like
> this can cause annoyances down the line. No merge issues this time
> (they often happen when duplicate patches then have other work done on
> top of them), but things like "git bisect" now don't have quite as
> black-and-white a situation etc etc.,
> 
> ("git bisect" will still find _one_ of the duplicate commits if it
> introduced a problem, so it's usually not a huge deal, but it can
> cause the bug to be then repeated if people revert that one, but
> nobody ever notices that the other commit that did the same thing is
> still around and it gets back-ported to stable or whatever..)
> 
> So part of this is just in general about confusing duplicate history,
> and part of it is that the duplication can then cause later confusion.

Urgh, sorry.  I /was/ careful to make sure the patches matched, but I'll
be more careful the next time this (hopefully never) happens again. :/

--D


>                 Linus