| Message ID | 20190422004849.26463-2-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support MAP_SYNC for memory-backend-file | expand |
On Mon, Apr 22, 2019 at 08:48:48AM +0800, Wei Yang wrote: > From: Zhang Yi <yi.z.zhang@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.zhang@intel.com> > Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com> > [ehabkost: Rebased patch to latest code on master] > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > Tested-by: Wei Yang <richardw.yang@linux.intel.com> > > --- > v14: rebase on top of current upstream > --- > util/mmap-alloc.c | 41 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c > index 9713f4b960..f7f177d0ea 100644 > --- a/util/mmap-alloc.c > +++ b/util/mmap-alloc.c > @@ -10,6 +10,13 @@ > * later. See the COPYING file in the top-level directory. > */ > > +#ifdef CONFIG_LINUX > +#include <linux/mman.h> > +#else /* !CONFIG_LINUX */ > +#define MAP_SYNC 0x0 > +#define MAP_SHARED_VALIDATE 0x0 > +#endif /* CONFIG_LINUX */ MAP_SHARED_VALIDATE is is from 2017: commit 1c9725974074a047f6080eecc62c50a8e840d050 Author: Dan Williams <dan.j.williams@intel.com> Date: Wed Nov 1 16:36:30 2017 +0100 mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags This code fails to compile on Linux hosts with pre-4.15 headers. Instead you could use the following (even on Linux!): #ifndef MAP_SYNC #define MAP_SYNC 0 #endif #ifndef MAP_SHARED_VALIDATE #define MAP_SHARED_VALIDATE 0 #endif Either way: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Tue, Apr 23, 2019 at 10:25:18AM +0100, Stefan Hajnoczi wrote: [...] >> +#ifdef CONFIG_LINUX >> +#include <linux/mman.h> >> +#else /* !CONFIG_LINUX */ >> +#define MAP_SYNC 0x0 >> +#define MAP_SHARED_VALIDATE 0x0 >> +#endif /* CONFIG_LINUX */ > >MAP_SHARED_VALIDATE is is from 2017: > > commit 1c9725974074a047f6080eecc62c50a8e840d050 > Author: Dan Williams <dan.j.williams@intel.com> > Date: Wed Nov 1 16:36:30 2017 +0100 > > mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags > >This code fails to compile on Linux hosts with pre-4.15 headers. > Ok, qemu build will fail on pre-4.15 linux. >Instead you could use the following (even on Linux!): > > #ifndef MAP_SYNC > #define MAP_SYNC 0 > #endif > #ifndef MAP_SHARED_VALIDATE > #define MAP_SHARED_VALIDATE 0 > #endif > >Either way: You mean replace the above code to: #ifdef CONFIG_LINUX #include <linux/mman.h> #endif /* CONFIG_LINUX */ #ifndef MAP_SYNC #define MAP_SYNC 0 #endif #ifndef MAP_SHARED_VALIDATE #define MAP_SHARED_VALIDATE 0 #endif > >Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Wed, Apr 24, 2019 at 09:01:05AM +0800, Wei Yang wrote: > On Tue, Apr 23, 2019 at 10:25:18AM +0100, Stefan Hajnoczi wrote: > [...] > >> +#ifdef CONFIG_LINUX > >> +#include <linux/mman.h> > >> +#else /* !CONFIG_LINUX */ > >> +#define MAP_SYNC 0x0 > >> +#define MAP_SHARED_VALIDATE 0x0 > >> +#endif /* CONFIG_LINUX */ > > > >MAP_SHARED_VALIDATE is is from 2017: > > > > commit 1c9725974074a047f6080eecc62c50a8e840d050 > > Author: Dan Williams <dan.j.williams@intel.com> > > Date: Wed Nov 1 16:36:30 2017 +0100 > > > > mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags > > > >This code fails to compile on Linux hosts with pre-4.15 headers. > > > > Ok, qemu build will fail on pre-4.15 linux. > > >Instead you could use the following (even on Linux!): > > > > #ifndef MAP_SYNC > > #define MAP_SYNC 0 > > #endif > > #ifndef MAP_SHARED_VALIDATE > > #define MAP_SHARED_VALIDATE 0 > > #endif > > > >Either way: > > You mean replace the above code to: > > #ifdef CONFIG_LINUX > #include <linux/mman.h> > #endif /* CONFIG_LINUX */ > > #ifndef MAP_SYNC > #define MAP_SYNC 0 > #endif > #ifndef MAP_SHARED_VALIDATE > #define MAP_SHARED_VALIDATE 0 > #endif Yes, please. This is not critical but I bet someone will hit this issue when compiling a new QEMU on an old host. This small tweak will save them time. Stefan
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 9713f4b960..f7f177d0ea 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -10,6 +10,13 @@ * later. See the COPYING file in the top-level directory. */ +#ifdef CONFIG_LINUX +#include <linux/mman.h> +#else /* !CONFIG_LINUX */ +#define MAP_SYNC 0x0 +#define MAP_SHARED_VALIDATE 0x0 +#endif /* CONFIG_LINUX */ + #include "qemu/osdep.h" #include "qemu/mmap-alloc.h" #include "qemu/host-utils.h" @@ -82,6 +89,7 @@ void *qemu_ram_mmap(int fd, bool is_pmem) { int flags; + int map_sync_flags = 0; int guardfd; size_t offset; size_t pagesize; @@ -132,9 +140,40 @@ void *qemu_ram_mmap(int fd, flags = MAP_FIXED; flags |= fd == -1 ? MAP_ANONYMOUS : 0; flags |= shared ? MAP_SHARED : MAP_PRIVATE; + if (shared && is_pmem) { + map_sync_flags = MAP_SYNC | MAP_SHARED_VALIDATE; + } + offset = QEMU_ALIGN_UP((uintptr_t)guardptr, align) - (uintptr_t)guardptr; - ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, flags, fd, 0); + ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, + flags | map_sync_flags, fd, 0); + + if (ptr == MAP_FAILED && map_sync_flags) { + if (errno == ENOTSUP) { + char *proc_link, *file_name; + int len; + proc_link = g_strdup_printf("/proc/self/fd/%d", fd); + file_name = g_malloc0(PATH_MAX); + len = readlink(proc_link, file_name, PATH_MAX - 1); + if (len < 0) { + len = 0; + } + file_name[len] = '\0'; + fprintf(stderr, "Warning: requesting persistence across crashes " + "for backend file %s failed. Proceeding without " + "persistence, data might become corrupted in case of host " + "crash.\n", file_name); + g_free(proc_link); + g_free(file_name); + } + /* + * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC, + * we will remove these flags to handle compatibility. + */ + ptr = mmap(guardptr + offset, size, PROT_READ | PROT_WRITE, + flags, fd, 0); + } if (ptr == MAP_FAILED) { munmap(guardptr, total);