On Thu, Feb 07, 2019 at 01:27:19AM +0800, Zhang, Yi wrote: > From: Zhang Yi <yi.z.zh...@linux.intel.com> > > 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). > > Current, We have below different possible use cases: > > 1. pmem=on is set, shared=on is set, MAP_SYNC supported: > a: backend is a dax supporting file. > - MAP_SYNC will active. > b: backend is not a dax supporting file. > - mmap will trigger a warning. then MAP_SYNC flag will be ignored > > 2. The rest of cases: > - we will never pass the MAP_SYNC to mmap2 > > Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com> > Signed-off-by: Zhang Yi <yi.z.zh...@linux.intel.com> > --- > include/qemu/osdep.h | 7 +++++++ > util/mmap-alloc.c | 24 +++++++++++++++++++++++- > 2 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 457d24e..9a94cc3 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -115,6 +115,13 @@ extern int daemon(int, int); > #include "sysemu/os-win32.h" > #endif > > +#ifdef CONFIG_LINUX > +#include <linux/mman.h> > +#else /* !CONFIG_LINUX */ > +#define MAP_SYNC 0x0 > +#define MAP_SHARED_VALIDATE 0x0 > +#endif /* CONFIG_LINUX */ > + > #ifdef CONFIG_POSIX > #include "sysemu/os-posix.h" > #endif
It's only used in one place. Maybe put this code in mmap-alloc.c ? > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 97bbeed..e4e55fc 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -15,6 +15,7 @@ > #include "qemu/host-utils.h" > > #define HUGETLBFS_MAGIC 0x958458f6 > +#define MAP_SYNC_FLAGS (MAP_SYNC | MAP_SHARED_VALIDATE) > Pls don't do this, just put it in a local variable within qemu_ram_mmap. > #ifdef CONFIG_LINUX > #include <sys/vfs.h> > @@ -101,6 +102,7 @@ void *qemu_ram_mmap(int fd, > #else > void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, > 0); > #endif > + int mmap_flags; > size_t offset; > void *ptr1; > > @@ -111,13 +113,33 @@ void *qemu_ram_mmap(int fd, > assert(is_power_of_2(align)); > /* Always align to host page size */ > assert(align >= getpagesize()); > + mmap_flags = shared ? MAP_SHARED : MAP_PRIVATE; > + if (shared && is_pmem) { > + mmap_flags |= MAP_SYNC_FLAGS; > + } > > offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr; > ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE, > MAP_FIXED | > (fd == -1 ? MAP_ANONYMOUS : 0) | > - (shared ? MAP_SHARED : MAP_PRIVATE), > + mmap_flags, > fd, 0); > + > + > + if (ptr1 == MAP_FAILED && > + (mmap_flags & MAP_SYNC_FLAGS) == MAP_SYNC_FLAGS) { > + if (errno == ENOTSUP) { > + perror("failed to validate with mapping flags"); I don't think this warning message makes sense. Are you trying to say: Warning: requesting persistence across crashes for file XYZ failed. Proceeding without persistence, data might become corrupted in case of host crash. ? > + } > + /* if map failed with MAP_SHARED_VALIDATE | MAP_SYNC, > + * we will remove these flags to handle compatibility. > + */ > + ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE, > + MAP_FIXED | > + (fd == -1 ? MAP_ANONYMOUS : 0) | > + MAP_SHARED, > + fd, 0); > + } > if (ptr1 == MAP_FAILED) { > munmap(ptr, total); > return MAP_FAILED; > -- > 2.7.4