Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1217441
| From | "Kirill A. Shutemov" <kirill@shutemov.name> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] dax, pmem: add support for msync |
| Date | 2015-09-02 11:20 +0200 |
| Message-ID | <q4cn1-KF-23@gated-at.bofh.it> (permalink) |
| References | <q3CCS-8jt-11@gated-at.bofh.it> <q3GQa-5Ns-7@gated-at.bofh.it> <q3QFT-3qt-83@gated-at.bofh.it> <q42xl-3mq-51@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, Sep 02, 2015 at 08:49:22AM +1000, Dave Chinner wrote:
> On Tue, Sep 01, 2015 at 01:08:04PM +0300, Kirill A. Shutemov 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:
> > > 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.
> >
> > Why?
> > IIUC, msync() doesn't have any requirements wrt metadata, right?
>
> Of course it does. If the backing store allocation has not been
> committed, then after a crash there will be a hole in file and
> so it will read as zeroes regardless of what data was written and
> flushed.
Any reason why backing store allocation cannot be committed on *_mkwrite?
> > > I pointed this out almost 6 months ago (i.e. that fsync was broken)
> > > anf hinted at how to solve it. Fix fsync, and msync gets fixed for
> > > free:
> > >
> > > https://lists.01.org/pipermail/linux-nvdimm/2015-March/000341.html
> > >
> > > I've also reported to Willy that DAX write page faults don't work
> > > correctly, either. xfstests generic/080 exposes this: a read
> > > from a page followed immediately by a write to that page does not
> > > result in ->page_mkwrite being called on the write and so
> > > backing store is not allocated for the page, nor are the timestamps
> > > for the file updated. This will also result in fsync (and msync)
> > > not working properly.
> >
> > Is that because XFS doesn't provide vm_ops->pfn_mkwrite?
>
> I didn't know that had been committed. I don't recall seeing a pull
> request with that in it
It went though -mm tree.
> none of the XFS DAX patches conflicted
> against it and there's been no runtime errors. I'll fix it up.
>
> As such, shouldn't there be a check in the VM (in ->mmap callers)
> that if we have the vma is returned with VM_MIXEDMODE enabled that
> ->pfn_mkwrite is not NULL? It's now clear to me that any filesystem
> that sets VM_MIXEDMODE needs to support both page_mkwrite and
> pfn_mkwrite, and such a check would have caught this immediately...
I guess it's "both or none" case. We have VM_MIXEDMAP users who don't care
about *_mkwrite.
I'm not yet sure it would be always correct, but something like this will
catch the XFS case, without false-positive on other stuff in my KVM setup:
diff --git a/mm/mmap.c b/mm/mmap.c
index 3f78bceefe5a..f2e29a541e14 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1645,6 +1645,15 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
vma->vm_ops = &dummy_ops;
}
+ /*
+ * Make sure that for VM_MIXEDMAP VMA has both
+ * vm_ops->page_mkwrite and vm_ops->pfn_mkwrite or has none.
+ */
+ if ((vma->vm_ops->page_mkwrite || vma->vm_ops->pfn_mkwrite) &&
+ vma->vm_flags & VM_MIXEDMAP) {
+ VM_BUG_ON_VMA(!vma->vm_ops->page_mkwrite, vma);
+ VM_BUG_ON_VMA(!vma->vm_ops->pfn_mkwrite, vma);
+ }
addr = vma->vm_start;
vm_flags = vma->vm_flags;
} else if (vm_flags & VM_SHARED) {
--
Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-08-31 21:10 +0200
Re: [PATCH] dax, pmem: add support for msync Christoph Hellwig <hch@lst.de> - 2015-08-31 21:10 +0200
Re: [PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-08-31 21:30 +0200
Re: [PATCH] dax, pmem: add support for msync Christoph Hellwig <hch@lst.de> - 2015-08-31 21:40 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Chinner <david@fromorbit.com> - 2015-09-01 01:40 +0200
Re: [PATCH] dax, pmem: add support for msync Christoph Hellwig <hch@lst.de> - 2015-09-01 09:10 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-01 14:20 +0200
Re: [PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-09-02 21:10 +0200
Re: [PATCH] dax, pmem: add support for msync "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-02 22:20 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-03 08:40 +0200
Re: [PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-09-03 18:50 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Chinner <david@fromorbit.com> - 2015-09-02 00:30 +0200
Re: [PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-09-02 05:20 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Chinner <david@fromorbit.com> - 2015-09-02 07:20 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 12:30 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Hansen <dave.hansen@linux.intel.com> - 2015-09-02 16:30 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 17:20 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Hansen <dave.hansen@linux.intel.com> - 2015-09-02 17:50 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 18:10 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Hansen <dave.hansen@linux.intel.com> - 2015-09-02 18:20 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-03 08:50 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 12:10 +0200
Re: [PATCH] dax, pmem: add support for msync "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-01 12:10 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-01 13:30 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Chinner <david@fromorbit.com> - 2015-09-02 00:50 +0200
Re: [PATCH] dax, pmem: add support for msync "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-02 11:20 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 11:40 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 11:50 +0200
Re: [PATCH] dax, pmem: add support for msync "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-09-02 11:50 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-02 12:30 +0200
Re: [PATCH] dax, pmem: add support for msync Dave Chinner <david@fromorbit.com> - 2015-09-03 03:00 +0200
Re: [PATCH] dax, pmem: add support for msync Boaz Harrosh <boaz@plexistor.com> - 2015-09-01 15:20 +0200
Re: [PATCH] dax, pmem: add support for msync Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-09-02 19:50 +0200
csiph-web