On Wed, Jan 02, 2019 at 01:26:15PM +0800, Zhang Yi wrote: > When a file supporting DAX is used as vNVDIMM backend, mmap it with > MAP_SYNC flag in addition which can ensure file system metadata > synced in each guest writes to the backend file, without other QEMU > actions (e.g., periodic fsync() by QEMU). > > Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com> > Signed-off-by: Zhang Yi <yi.z.zh...@linux.intel.com> > --- > include/qemu/osdep.h | 16 ++++++++++++++++ > util/mmap-alloc.c | 12 +++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 3bf48bc..bb1eba1 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -410,6 +410,22 @@ void qemu_anon_ram_free(void *ptr, size_t size); > # define QEMU_VMALLOC_ALIGN getpagesize() > #endif > > +/* > + * MAP_SHARED_VALIDATE and MAP_SYNC are introduced in Linux kernel > + * 4.15, so they may not be defined when compiling on older kernels. > + */ > +#ifdef CONFIG_LINUX > + > +#include <asm-generic/mman.h> > + > +#ifndef MAP_SYNC > +#define MAP_SYNC 0x0 > +#endif > + > +#else /* !CONFIG_LINUX */ > +#define MAP_SYNC 0x0 > +#endif /* CONFIG_LINUX */ > + > #ifdef CONFIG_POSIX > struct qemu_signalfd_siginfo { > uint32_t ssi_signo; /* Signal number */ > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 8f0a740..a9d5e56 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -99,6 +99,8 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, > uint32_t flags) > void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, > 0); > #endif > bool shared = flags & RAM_SHARED; > + bool is_pmem = flags & RAM_PMEM; > + int mmap_xflags = 0; > size_t offset; > void *ptr1; > > @@ -109,13 +111,21 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, > uint32_t flags) > assert(is_power_of_2(align)); > /* Always align to host page size */ > assert(align >= getpagesize()); > + if (shared && is_pmem) { > + mmap_xflags |= MAP_SYNC; > + } > > offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr; > + retry_mmap_fd: > ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE, > MAP_FIXED | > (fd == -1 ? MAP_ANONYMOUS : 0) | > - (shared ? MAP_SHARED : MAP_PRIVATE), > + (shared ? MAP_SHARED : MAP_PRIVATE) | mmap_xflags, > fd, 0); > + if ((ptr1 == MAP_FAILED) && (mmap_xflags & MAP_SYNC)) { > + mmap_xflags &= ~MAP_SYNC; > + goto retry_mmap_fd;
Do we have use cases where using pmem=on without MAP_SYNC isn't going to cause problems? If not, shouldn't we at least print a warning here? Otherwise, won't we still need an option for cases that require MAP_SYNC to be working? > + } -- Eduardo