linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Jan Kara <jack@suse.cz>
Cc: ira.weiny@intel.com, linux-kernel@vger.kernel.org,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Jeff Moyer <jmoyer@redhat.com>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH V6 7/8] fs/xfs: Change xfs_ioctl_setattr_dax_invalidate() to xfs_ioctl_dax_check()
Date: Thu, 9 Apr 2020 07:09:50 +1000	[thread overview]
Message-ID: <20200408210950.GL24067@dread.disaster.area> (raw)
In-Reply-To: <20200408095803.GB30172@quack2.suse.cz>

On Wed, Apr 08, 2020 at 11:58:03AM +0200, Jan Kara wrote:
> On Wed 08-04-20 12:23:18, Dave Chinner wrote:
> > On Tue, Apr 07, 2020 at 11:29:57AM -0700, ira.weiny@intel.com wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > > 
> > > We only support changing FS_XFLAG_DAX on directories.  Files get their
> > > flag from the parent directory on creation only.  So no data
> > > invalidation needs to happen.
> > 
> > Which leads me to ask: how are users and/or admins supposed to
> > remove the flag from regular files once it is set in the filesystem?
> > 
> > Only being able to override the flag via the "dax=never" mount
> > option means that once the flag is set, nobody can ever remove it
> > and they can only globally turn off dax if it gets set incorrectly.
> > It also means a global interrupt because all apps on the filesystem
> > need to be stopped so the filesystem can be unmounted and mounted
> > again with dax=never. This is highly unfriendly to admins and users.
> > 
> > IOWs, we _must_ be able to clear this inode flag on regular inodes
> > in some way. I don't care if it doesn't change the current in-memory
> > state, but we must be able to clear the flags so that the next time
> > the inodes are instantiated DAX is not enabled for those files...
> 
> Well, there's one way to clear the flag: delete the file. If you still care
> about the data, you can copy the data first. It isn't very convenient, I
> agree, and effectively means restarting whatever application that is using
> the file.

Restarting the application is fine. Having to backup/restore or copy
the entire data set just to turn off an inode flag? That's not a
viable management strategy. We could be talking about terabytes of
data here.

I explained how we can safely remove the flag in the other branch of
this thread...

> But it seems like more understandable API than letting user clear
> the on-disk flag but the inode will still use DAX until kernel decides to
> evict the inode

Certainly doesn't seem that way to me. "stop app, clear flags, drop
caches, restart app" is a pretty simple, easy thing to do for an
admin.

Especially compared to process that is effectively "stop app, backup
data set, delete data set, clear flags, restore data set, restart
app"

> - because that often means you need to restart the
> application using the file anyway for the flag change to have any effect.

That's a trivial requirement compared to the downtime and resource
cost of a data set backup/restore just to clear inode flags....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2020-04-08 21:09 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-07 18:29 [PATCH V6 0/8] Enable per-file/per-directory DAX operations V6 ira.weiny
2020-04-07 18:29 ` [PATCH V6 1/8] fs/xfs: Remove unnecessary initialization of i_rwsem ira.weiny
2020-04-07 23:46   ` Dave Chinner
2020-04-07 18:29 ` [PATCH V6 2/8] fs: Remove unneeded IS_DAX() check ira.weiny
2020-04-09  7:31   ` Christoph Hellwig
2020-04-09 14:57     ` Ira Weiny
2020-04-07 18:29 ` [PATCH V6 3/8] fs/stat: Define DAX statx attribute ira.weiny
2020-04-07 23:47   ` Dave Chinner
2020-04-07 18:29 ` [PATCH V6 4/8] fs/xfs: Make DAX mount option a tri-state ira.weiny
2020-04-07 23:59   ` Dave Chinner
2020-04-08  0:09     ` Ira Weiny
2020-04-08  0:48       ` Dave Chinner
2020-04-09 15:03         ` Ira Weiny
2020-04-07 18:29 ` [PATCH V6 5/8] fs/xfs: Create function xfs_inode_enable_dax() ira.weiny
2020-04-08  0:05   ` Dave Chinner
2020-04-08  0:13     ` Ira Weiny
2020-04-07 18:29 ` [PATCH V6 6/8] fs/xfs: Combine xfs_diflags_to_linux() and xfs_diflags_to_iflags() ira.weiny
2020-04-08  2:08   ` Dave Chinner
2020-04-08 17:09     ` Ira Weiny
2020-04-08 21:02       ` Dave Chinner
2020-04-08 21:28         ` Dan Williams
2020-04-08 22:10           ` Ira Weiny
2020-04-08 23:58           ` Dave Chinner
2020-04-09  0:22             ` Ira Weiny
2020-04-09 12:41               ` Christoph Hellwig
2020-04-09 20:49                 ` Ira Weiny
2020-04-08 22:07         ` Ira Weiny
2020-04-08 23:21           ` Dave Chinner
2020-04-09  0:12             ` Ira Weiny
2020-04-09  0:30               ` Darrick J. Wong
2020-04-09 15:29                 ` Ira Weiny
2020-04-09 16:59                   ` Darrick J. Wong
2020-04-09 17:17                     ` Jan Kara
2020-04-09 20:54                     ` Ira Weiny
2020-04-09  0:49               ` Dave Chinner
2020-04-09 12:40                 ` Christoph Hellwig
2020-04-10  0:27                   ` Dave Chinner
2020-04-07 18:29 ` [PATCH V6 7/8] fs/xfs: Change xfs_ioctl_setattr_dax_invalidate() to xfs_ioctl_dax_check() ira.weiny
2020-04-08  2:23   ` Dave Chinner
2020-04-08  9:58     ` Jan Kara
2020-04-08 21:09       ` Dave Chinner [this message]
2020-04-08 22:26         ` Ira Weiny
2020-04-08 23:48           ` Dave Chinner
2020-04-09 12:28             ` Christoph Hellwig
2020-04-08 15:37   ` Darrick J. Wong
2020-04-08 18:13     ` Ira Weiny
2020-04-16  5:39   ` [fs/xfs] 857c9841f8: xfstests.xfs.046.fail kernel test robot
2020-04-07 18:29 ` [PATCH V6 8/8] Documentation/dax: Update Usage section ira.weiny

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=20200408210950.GL24067@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=hch@lst.de \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=jmoyer@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=tytso@mit.edu \
    --subject='Re: [PATCH V6 7/8] fs/xfs: Change xfs_ioctl_setattr_dax_invalidate() to xfs_ioctl_dax_check()' \
    /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

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).