linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Andreas Dilger <adilger@dilger.ca>,
	Dan Williams <dan.j.williams@intel.com>,
	Eric Sandeen <sandeen@redhat.com>,
	Lukas Czerner <lczerner@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	"Theodore Ts'o" <tytso@mit.edu>, Christoph Hellwig <hch@lst.de>,
	Jan Kara <jack@suse.cz>, linux-ext4 <linux-ext4@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH 0/9] add ext4 per-inode DAX flag
Date: Thu, 7 Sep 2017 16:19:00 -0600	[thread overview]
Message-ID: <20170907221900.GB12669@linux.intel.com> (raw)
In-Reply-To: <20170907221201.GZ17782@dastard>

On Fri, Sep 08, 2017 at 08:12:01AM +1000, Dave Chinner wrote:
> On Thu, Sep 07, 2017 at 03:51:48PM -0600, Ross Zwisler wrote:
> > On Thu, Sep 07, 2017 at 03:26:10PM -0600, Andreas Dilger wrote:
> > > However, I wonder if this could
> > > be prevented at runtime, and only allow S_DAX to be set when the inode is
> > > first instantiated, and wouldn't be allowed to change after that?  Setting
> > > or clearing the per-inode DAX flag might still be allowed, but it wouldn't
> > > be enabled until the inode is next fetched into cache?  Similarly, for
> > > inodes that have conflicting features (e.g. inline data or encryption)
> > > would not be allowed to enable S_DAX.
> > 
> > Ooh, this seems interesting.  This would ensure that S_DAX transitions
> > couldn't ever race with I/Os or mmaps().  I had some other ideas for how to
> > handle this, but I think your idea is more promising. :)
> 
> IMO, that's an awful admin interface - it can't be done on demand
> (i.e. when needed) because we can't force an inode to be evicted
> from the cache. And then we have the "why the hell did that just
> change" problem if an inode is evicted due to memory pressure and
> then immediately reinstantiated by the running workload. That's a
> recipe for driving admins insane...
> 
> > I guess with this solution we'd need:
> > 
> > a) A good way of letting the user detect the state where they had set the DAX
> > inode flag, but that it wasn't yet in use by the inode.
> > 
> > b) A reliable way of flushing the inode from the filesystem cache, so that the
> > next time an open() happens they get the new behavior.  The way I usually do
> > this is via umount/remount, but there is probably already a way to do this?
> 
> Not if it's referenced. And if it's not referenced, then the only
> hammer we have is Brutus^Wdrop_caches. That's not an option for
> production machines.
> 
> Neat idea, but one I'd already thought of and discarded as "not
> practical from an admin perspective".

Okay, so other ideas (which you have also probably already though of) include:

1) Just return -EBUSY if anyone tries to change the DAX flag of an inode with
open mappings or any open file handles.  To prevent TOCTOU races we'd have to
do some additional locking while actually changing the flag.

2) Be more drastic and follow the flow of ext4 file based encryption, only
allowing the inode flag to be set by an admin on an empty directory.  Files in
that directory will inherit it when they are created, and we don't provide a
way to clear.  If you want your file to not use DAX, move it to a different
directory (which I think for ext4 encryption turns it into a new inode).

Other ideas?

  reply	other threads:[~2017-09-07 22:19 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-05 22:35 [PATCH 0/9] add ext4 per-inode DAX flag Ross Zwisler
2017-09-05 22:35 ` [PATCH 1/9] ext4: remove duplicate extended attributes defs Ross Zwisler
2017-09-06  7:29   ` Jan Kara
2017-09-05 22:35 ` [PATCH 2/9] xfs: always use DAX if mount option is used Ross Zwisler
2017-09-05 22:35 ` [PATCH 3/9] xfs: validate bdev support for DAX inode flag Ross Zwisler
2017-09-05 22:35 ` [PATCH 4/9] ext4: add ext4_should_use_dax() Ross Zwisler
2017-09-05 22:35 ` [PATCH 5/9] ext4: ext4_change_inode_journal_flag error handling Ross Zwisler
2017-09-05 22:35 ` [PATCH 6/9] ext4: safely transition S_DAX on journaling changes Ross Zwisler
2017-09-06  9:47   ` Jan Kara
2017-09-06 17:09     ` Ross Zwisler
2017-09-05 22:35 ` [PATCH 7/9] ext4: prevent data corruption with inline data + DAX Ross Zwisler
2017-09-06 20:55   ` Andreas Dilger
2017-09-06 23:11     ` Ross Zwisler
2017-09-05 22:35 ` [PATCH 8/9] ext4: add sanity check for encryption " Ross Zwisler
2017-09-05 22:35 ` [PATCH 9/9] ext4: add per-inode DAX flag Ross Zwisler
2017-09-06  2:12 ` [PATCH 0/9] add ext4 " Eric Sandeen
2017-09-06 17:07   ` Ross Zwisler
2017-09-07 20:54     ` Dan Williams
2017-09-07 21:13       ` Ross Zwisler
2017-09-07 21:26         ` Andreas Dilger
2017-09-07 21:51           ` Ross Zwisler
2017-09-07 22:12             ` Dave Chinner
2017-09-07 22:19               ` Ross Zwisler [this message]
2017-09-07 23:25                 ` Dave Chinner
2017-09-08  9:48                   ` Jan Kara
2017-09-08 15:39                   ` Theodore Ts'o
2017-09-11  8:47                     ` Jan Kara

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=20170907221900.GB12669@linux.intel.com \
    --to=ross.zwisler@linux.intel.com \
    --cc=adilger@dilger.ca \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --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).