linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@osdl.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Hugh Dickins <hughd@google.com>,
	Ingo Molnar <mingo@redhat.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-nvdimm@lists.01.org, Matthew Wilcox <willy@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	x86@kernel.org
Subject: Re: [PATCH] dax, pmem: add support for msync
Date: Wed, 2 Sep 2015 08:21:20 +1000	[thread overview]
Message-ID: <20150901222120.GQ3902@dastard> (raw)
In-Reply-To: <20150901070608.GA5482@lst.de>

On Tue, Sep 01, 2015 at 09:06:08AM +0200, Christoph Hellwig wrote:
> On Tue, Sep 01, 2015 at 09:38:03AM +1000, Dave Chinner wrote:
> > On Mon, Aug 31, 2015 at 12:59:44PM -0600, Ross Zwisler wrote:
> > > For DAX msync we just need to flush the given range using
> > > wb_cache_pmem(), which is now a public part of the PMEM API.
> > 
> > This is wrong, because it still leaves fsync() broken on dax.
> > 
> > Flushing dirty data to stable storage is the responsibility of the
> > writeback infrastructure, not the VMA/mm infrasrtucture. For non-dax
> > configurations, msync defers all that to vfs_fsync_range(), because
> > it has to be implemented there for fsync() to work.
> > 
> > Even for DAX, msync has to call vfs_fsync_range() for the filesystem to commit
> > the backing store allocations to stable storage, so there's not
> > getting around the fact msync is the wrong place to be flushing
> > DAX mappings to persistent storage.
> 
> DAX does call ->fsync before and after this patch.  And with all
> the recent fixes we take care to ensure data is written though the
> cache for everything but mmap-access.  With this patch from Ross
> we ensure msync writes back the cache before calling ->fsync so that
> the filesystem can then do it's work like converting unwritten extents.
> 
> The only downside is that previously on Linux you could always use
> fsync as a replaement for msymc, which isn't true anymore for DAX.

Which means applications that should "just work" without
modification on DAX are now subtly broken and don't actually
guarantee data is safe after a crash. That's a pretty nasty
landmine, and goes against *everything* we've claimed about using
DAX with existing applications.

That's wrong, and needs fixing.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2015-09-01 22:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-31 18:59 [PATCH] dax, pmem: add support for msync Ross Zwisler
2015-08-31 19:06 ` Christoph Hellwig
2015-08-31 19:26   ` Ross Zwisler
2015-08-31 19:34     ` Christoph Hellwig
2015-08-31 23:38 ` Dave Chinner
2015-09-01  7:06   ` Christoph Hellwig
2015-09-01 12:18     ` Boaz Harrosh
2015-09-02 19:04       ` Ross Zwisler
2015-09-02 20:17         ` Kirill A. Shutemov
2015-09-03  6:32         ` Boaz Harrosh
2015-09-03 16:44           ` Ross Zwisler
2015-09-01 22:21     ` Dave Chinner [this message]
2015-09-02  3:19       ` Ross Zwisler
2015-09-02  5:17         ` Dave Chinner
2015-09-02 10:27           ` Boaz Harrosh
2015-09-02 14:23             ` Dave Hansen
2015-09-02 15:18               ` Boaz Harrosh
2015-09-02 15:39                 ` Dave Hansen
2015-09-02 16:00                   ` Boaz Harrosh
2015-09-02 16:19                     ` Dave Hansen
2015-09-03  6:41                       ` Boaz Harrosh
2015-09-02 10:04         ` Boaz Harrosh
2015-09-01 10:08   ` Kirill A. Shutemov
2015-09-01 11:27     ` Boaz Harrosh
2015-09-01 22:49     ` Dave Chinner
2015-09-02  9:13       ` Kirill A. Shutemov
2015-09-02  9:37         ` Boaz Harrosh
2015-09-02  9:41           ` Boaz Harrosh
2015-09-02  9:47             ` Kirill A. Shutemov
2015-09-02 10:28               ` Boaz Harrosh
2015-09-03  0:57         ` Dave Chinner
2015-09-01 13:12 ` Boaz Harrosh
2015-09-02 17:47   ` Ross Zwisler

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=20150901222120.GQ3902@dastard \
    --to=david@fromorbit.com \
    --cc=akpm@osdl.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=hughd@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ross.zwisler@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@linux.intel.com \
    --cc=x86@kernel.org \
    /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).