* [Qemu-devel] [PATCH v2 0/2] nvdimm: support MAP_SYNC for memory-backend-file
@ 2018-01-11 14:22 Haozhong Zhang
2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 1/2] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() Haozhong Zhang
2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option Haozhong Zhang
0 siblings, 2 replies; 5+ messages in thread
From: Haozhong Zhang @ 2018-01-11 14:22 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Igor Mammedov, Paolo Bonzini, mst,
Xiao Guangrong, Stefan Hajnoczi, Dan Williams, Haozhong Zhang
v2 is based on Eduardo's machine-next tree. Changes of
memory-backend-file are based on my previous patches in that tree.
Linux 4.15 introduces a new mmap flag MAP_SYNC, which can be used to
guarantee the write persistence to mmap'ed files supporting DAX (e.g.,
files on ext4/xfs file system mounted with '-o dax').
This patchset enables QEMU to use MAP_SYNC flag for memory-backend-file,
in order to guarantee the guest write persistence to backend files
supporting DAX.
A new auto on/off option 'sync' is added to memory-backend-file:
- on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or
'share=off', QEMU will abort
- off: never pass MAP_SYNC to mmap(2)
- auto (default): if MAP_SYNC is supported and 'share=on', work as if
'sync=on'; otherwise, work as if 'sync=off'
Changes in v2:
* Add 'sync' option to control the use of MAP_SYNC. (Eduardo Habkost)
* Remove the unnecessary set of MAP_SHARED_VALIDATE in some cases and
the retry mechanism in qemu_ram_mmap(). (Michael S. Tsirkin)
* Move OS dependent definitions of MAP_SYNC and MAP_SHARED_VALIDATE
to osdep.h. (Michael S. Tsirkin)
Haozhong Zhang (2):
util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()
hostmem-file: add 'sync' option
backends/hostmem-file.c | 39 ++++++++++++++++++++++++++++++++++++++-
docs/nvdimm.txt | 15 ++++++++++++++-
exec.c | 13 ++++++++-----
include/exec/memory.h | 4 ++++
include/exec/ram_addr.h | 6 +++---
include/qemu/mmap-alloc.h | 3 ++-
include/qemu/osdep.h | 16 ++++++++++++++++
memory.c | 6 ++++--
numa.c | 2 +-
qemu-options.hx | 21 ++++++++++++++++++++-
util/mmap-alloc.c | 24 ++++++++++++++++++++++--
util/oslib-posix.c | 2 +-
12 files changed, 133 insertions(+), 18 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH v2 1/2] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() 2018-01-11 14:22 [Qemu-devel] [PATCH v2 0/2] nvdimm: support MAP_SYNC for memory-backend-file Haozhong Zhang @ 2018-01-11 14:22 ` Haozhong Zhang 2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option Haozhong Zhang 1 sibling, 0 replies; 5+ messages in thread From: Haozhong Zhang @ 2018-01-11 14:22 UTC (permalink / raw) To: qemu-devel Cc: Eduardo Habkost, Igor Mammedov, Paolo Bonzini, mst, Xiao Guangrong, Stefan Hajnoczi, Dan Williams, Haozhong Zhang When a file supporting DAX is used as vNVDIMM backend, mmap it with MAP_SYNC flag in addition can guarantee the persistence of guest write to the backend file without other QEMU actions (e.g., periodic fsync() by QEMU). A OnOffAuto parameter 'sync' is added to qemu_ram_mmap(): - If sync == ON_OFF_AUTO_ON, qemu_ram_mmap() will try to pass MAP_SYNC to mmap(). It will then fail if the host OS or the backend file do not support MAP_SYNC, or MAP_SYNC is conflict with other flags. - If sync == ON_OFF_AUTO_OFF, qemu_ram_mmap() will never pass MAP_SYNC to mmap(). - If sync == ON_OFF_AUTO_AUTO, and * if the host OS and the backend file support MAP_SYNC, and MAP_SYNC is not conflict with other flags, qemu_ram_mmap() will work as if sync == ON_OFF_AUTO_ON. * otherwise, qemu_ram_mmap() will work as if sync == ON_OFF_AUTO_OFF. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- exec.c | 2 +- include/qemu/mmap-alloc.h | 3 ++- include/qemu/osdep.h | 16 ++++++++++++++++ util/mmap-alloc.c | 24 ++++++++++++++++++++++-- util/oslib-posix.c | 2 +- 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/exec.c b/exec.c index 8fba88ae1c..f4254cb6d3 100644 --- a/exec.c +++ b/exec.c @@ -1646,7 +1646,7 @@ static void *file_ram_alloc(RAMBlock *block, } area = qemu_ram_mmap(fd, memory, block->mr->align, - block->flags & RAM_SHARED); + block->flags & RAM_SHARED, ON_OFF_AUTO_OFF); if (area == MAP_FAILED) { error_setg_errno(errp, errno, "unable to map backing store for guest RAM"); diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h index 50385e3f81..dd5876471f 100644 --- a/include/qemu/mmap-alloc.h +++ b/include/qemu/mmap-alloc.h @@ -7,7 +7,8 @@ size_t qemu_fd_getpagesize(int fd); size_t qemu_mempath_getpagesize(const char *mem_path); -void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared); +void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared, + OnOffAuto sync); void qemu_ram_munmap(void *ptr, size_t size); diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index adb3758275..55637e0724 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -372,6 +372,22 @@ void qemu_anon_ram_free(void *ptr, size_t size); # define QEMU_VMALLOC_ALIGN getpagesize() #endif +/* + * MAP_SHARED_VALIDATE and MAP_SYNC were introduced in Linux kernel + * 4.15, so they may not be defined when compiling on older kernels. + */ +#ifdef CONFIG_LINUX +#ifndef MAP_SHARED_VALIDATE +#define MAP_SHARED_VALIDATE 0x3 +#endif +#ifndef MAP_SYNC +#define MAP_SYNC 0x80000 +#endif +#define QEMU_HAS_MAP_SYNC true +#else /* !CONFIG_LINUX */ +#define QEMU_HAS_MAP_SYNC false +#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 2fd8cbcc6f..af57218669 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -73,7 +73,8 @@ size_t qemu_mempath_getpagesize(const char *mem_path) return getpagesize(); } -void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared) +void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared, + OnOffAuto sync) { /* * Note: this always allocates at least one extra page of virtual address @@ -97,6 +98,7 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared) #endif size_t offset; void *ptr1; + int xflags = 0; if (ptr == MAP_FAILED) { return MAP_FAILED; @@ -106,11 +108,29 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared) /* Always align to host page size */ assert(align >= getpagesize()); + if (!QEMU_HAS_MAP_SYNC || !shared) { + if (sync == ON_OFF_AUTO_ON) { + return MAP_FAILED; + } + sync = ON_OFF_AUTO_OFF; + } + if (sync != ON_OFF_AUTO_OFF) { + xflags = MAP_SYNC; + } + /* + * If MAP_SHARED_VALIDATE is present, mmap will fail when MAP_SYNC + * is not supported. Otherwise, mmap will just ignore MAP_SYNC when + * it's not supported. + */ + if (sync == ON_OFF_AUTO_ON) { + xflags |= MAP_SHARED_VALIDATE; + } + 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), + (shared ? MAP_SHARED : MAP_PRIVATE) | xflags, fd, 0); if (ptr1 == MAP_FAILED) { munmap(ptr, total); diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 77369c92ce..ecb1c275d2 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -130,7 +130,7 @@ void *qemu_memalign(size_t alignment, size_t size) void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment) { size_t align = QEMU_VMALLOC_ALIGN; - void *ptr = qemu_ram_mmap(-1, size, align, false); + void *ptr = qemu_ram_mmap(-1, size, align, false, ON_OFF_AUTO_OFF); if (ptr == MAP_FAILED) { return NULL; -- 2.15.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option 2018-01-11 14:22 [Qemu-devel] [PATCH v2 0/2] nvdimm: support MAP_SYNC for memory-backend-file Haozhong Zhang 2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 1/2] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() Haozhong Zhang @ 2018-01-11 14:22 ` Haozhong Zhang 2018-01-11 20:06 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 5+ messages in thread From: Haozhong Zhang @ 2018-01-11 14:22 UTC (permalink / raw) To: qemu-devel Cc: Eduardo Habkost, Igor Mammedov, Paolo Bonzini, mst, Xiao Guangrong, Stefan Hajnoczi, Dan Williams, Haozhong Zhang This option controls whether QEMU mmap(2) the memory backend file with MAP_SYNC flag, which can fully guarantee the guest write persistence to the backend, if MAP_SYNC flag is supported by the host kernel (Linux kernel 4.15 and later) and the backend is a file supporting DAX (e.g., file on ext4/xfs file system mounted with '-o dax'). It can take one of following values: - on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or 'share=off', QEMU will abort - off: never pass MAP_SYNC to mmap(2) - auto (default): if MAP_SYNC is supported and 'share=on', work as if 'sync=on'; otherwise, work as if 'sync=off' Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Sugguested-by: Eduardo Habkost <ehabkost@redhat.com> --- backends/hostmem-file.c | 39 ++++++++++++++++++++++++++++++++++++++- docs/nvdimm.txt | 15 ++++++++++++++- exec.c | 13 ++++++++----- include/exec/memory.h | 4 ++++ include/exec/ram_addr.h | 6 +++--- memory.c | 6 ++++-- numa.c | 2 +- qemu-options.hx | 21 ++++++++++++++++++++- 8 files changed, 92 insertions(+), 14 deletions(-) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index e319ec1ad8..52fdc0a48a 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -15,6 +15,7 @@ #include "sysemu/hostmem.h" #include "sysemu/sysemu.h" #include "qom/object_interfaces.h" +#include "qapi-visit.h" /* hostmem-file.c */ /** @@ -35,6 +36,7 @@ struct HostMemoryBackendFile { bool discard_data; char *mem_path; uint64_t align; + OnOffAuto sync; }; static void @@ -60,7 +62,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), path, backend->size, fb->align, fb->share, - fb->mem_path, errp); + fb->sync, fb->mem_path, errp); g_free(path); } #endif @@ -150,6 +152,38 @@ static void file_memory_backend_set_align(Object *o, Visitor *v, error_propagate(errp, local_err); } +static void file_memory_backend_get_sync( + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) +{ + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); + OnOffAuto value = fb->sync; + + visit_type_OnOffAuto(v, name, &value, errp); +} + +static void file_memory_backend_set_sync( + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) +{ + HostMemoryBackend *backend = MEMORY_BACKEND(obj); + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); + Error *local_err = NULL; + OnOffAuto value; + + if (host_memory_backend_mr_inited(backend)) { + error_setg(&local_err, "cannot change property value"); + goto out; + } + + visit_type_OnOffAuto(v, name, &value, &local_err); + if (local_err) { + goto out; + } + fb->sync = value; + + out: + error_propagate(errp, local_err); +} + static void file_backend_unparent(Object *obj) { HostMemoryBackend *backend = MEMORY_BACKEND(obj); @@ -184,6 +218,9 @@ file_backend_class_init(ObjectClass *oc, void *data) file_memory_backend_get_align, file_memory_backend_set_align, NULL, NULL, &error_abort); + object_class_property_add(oc, "sync", "OnOffAuto", + file_memory_backend_get_sync, file_memory_backend_set_sync, + NULL, NULL, &error_abort); } static void file_backend_instance_finalize(Object *o) diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt index e903d8bb09..49b174fe66 100644 --- a/docs/nvdimm.txt +++ b/docs/nvdimm.txt @@ -143,10 +143,23 @@ Guest Data Persistence ---------------------- Though QEMU supports multiple types of vNVDIMM backends on Linux, -currently the only one that can guarantee the guest write persistence +if MAP_SYNC is not supported by the host kernel and the backends, +the only backend that can guarantee the guest write persistence is the device DAX on the real NVDIMM device (e.g., /dev/dax0.0), to which all guest access do not involve any host-side kernel cache. +mmap(2) flag MAP_SYNC is added since Linux kernel 4.15. On such +systems, QEMU can mmap(2) the backend with MAP_SYNC, which can +guarantee the guest write persistence to vNVDIMM. Besides the host +kernel support, enabling MAP_SYNC in QEMU also requires: + + - the backend is a file supporting DAX, e.g., a file on an ext4 or + xfs file system mounted with "-o dax", + + - 'sync' option of memory-backend-file is not 'off', and + + - 'share' option of memory-backend-file is 'on'. + When using other types of backends, it's suggested to set 'unarmed' option of '-device nvdimm' to 'on', which sets the unarmed flag of the guest NVDIMM region mapping structure. This unarmed flag indicates diff --git a/exec.c b/exec.c index f4254cb6d3..ce13f8cb21 100644 --- a/exec.c +++ b/exec.c @@ -1600,6 +1600,7 @@ static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, int fd, bool truncate, + OnOffAuto sync, Error **errp) { void *area; @@ -1646,7 +1647,7 @@ static void *file_ram_alloc(RAMBlock *block, } area = qemu_ram_mmap(fd, memory, block->mr->align, - block->flags & RAM_SHARED, ON_OFF_AUTO_OFF); + block->flags & RAM_SHARED, sync); if (area == MAP_FAILED) { error_setg_errno(errp, errno, "unable to map backing store for guest RAM"); @@ -1974,7 +1975,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) #ifdef __linux__ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, - bool share, int fd, + bool share, OnOffAuto sync, int fd, Error **errp) { RAMBlock *new_block; @@ -2017,7 +2018,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, new_block->used_length = size; new_block->max_length = size; new_block->flags = share ? RAM_SHARED : 0; - new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp); + new_block->host = file_ram_alloc(new_block, size, fd, !file_size, sync, + errp); if (!new_block->host) { g_free(new_block); return NULL; @@ -2035,7 +2037,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, - bool share, const char *mem_path, + bool share, OnOffAuto sync, + const char *mem_path, Error **errp) { int fd; @@ -2047,7 +2050,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, return NULL; } - block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp); + block = qemu_ram_alloc_from_fd(size, mr, share, sync, fd, errp); if (!block) { if (created) { unlink(mem_path); diff --git a/include/exec/memory.h b/include/exec/memory.h index 07c5d6d597..ff3cd583e9 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -468,6 +468,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, * @align: alignment of the region base address; if 0, the default alignment * (getpagesize()) will be used. * @share: %true if memory must be mmaped with the MAP_SHARED flag + * @sync: %ON_OFF_AUTO_ON if memory must be mapped with MAP_SYNC flag; + * %ON_OFF_AUTO_OFF if memory cannot be mapped with MAP_SYNC flag; + * %ON_OFF_AUTO_AUTO directs QEMU to mmap with MAP_SYNC flag if possible * @path: the path in which to allocate the RAM. * @errp: pointer to Error*, to store an error if it happens. * @@ -480,6 +483,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, uint64_t size, uint64_t align, bool share, + OnOffAuto sync, const char *path, Error **errp); diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 6cbc02aa0f..4494ae9132 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -73,10 +73,10 @@ static inline unsigned long int ramblock_recv_bitmap_offset(void *host_addr, long qemu_getrampagesize(void); unsigned long last_ram_page(void); RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, - bool share, const char *mem_path, - Error **errp); + bool share, OnOffAuto sync, + const char *mem_path, Error **errp); RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, - bool share, int fd, + bool share, OnOffAuto sync, int fd, Error **errp); RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, MemoryRegion *mr, Error **errp); diff --git a/memory.c b/memory.c index 449a1429b9..e22f51394e 100644 --- a/memory.c +++ b/memory.c @@ -1572,6 +1572,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, uint64_t size, uint64_t align, bool share, + OnOffAuto sync, const char *path, Error **errp) { @@ -1580,7 +1581,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->align = align; - mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp); + mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, sync, path, errp); mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; } @@ -1596,7 +1597,8 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr, mr->ram = true; mr->terminates = true; mr->destructor = memory_region_destructor_ram; - mr->ram_block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp); + mr->ram_block = qemu_ram_alloc_from_fd(size, mr, share, ON_OFF_AUTO_OFF, fd, + errp); mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; } #endif diff --git a/numa.c b/numa.c index 83675a03f3..93180510a4 100644 --- a/numa.c +++ b/numa.c @@ -457,7 +457,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner, #ifdef __linux__ Error *err = NULL; memory_region_init_ram_from_file(mr, owner, name, ram_size, 0, false, - mem_path, &err); + ON_OFF_AUTO_OFF, mem_path, &err); if (err) { error_report_err(err); if (mem_prealloc) { diff --git a/qemu-options.hx b/qemu-options.hx index 5ff741a4af..3ee423e6a8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3974,7 +3974,7 @@ property must be set. These objects are placed in the @table @option -@item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},align=@var{align} +@item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},align=@var{align},sync=@var{on|off|auto} Creates a memory file backend object, which can be used to back the guest RAM with huge pages. @@ -4034,6 +4034,25 @@ requires an alignment different than the default one used by QEMU, eg the device DAX /dev/dax0.0 requires 2M alignment rather than 4K. In such cases, users can specify the required alignment via this option. +The @option{sync} option specifies whether QEMU mmap(2) @option{mem-path} +with MAP_SYNC flag, which can fully guarantee the guest write +persistence to @option{mem-path}. MAP_SYNC requires supports from both +the host kernel (since Linux kernel 4.15) and @option{mem-path} (only +files supporting DAX). It can take one of following values: + +@table @option +@item @var{on} +try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or +@option{share}=@var{off}, QEMU will abort + +@item @var{off} +never pass MAP_SYNC to mmap(2) + +@item @var{auto} (default) +if MAP_SYNC is supported and @option{share}=@var{on}, work as if +@option{sync}=@var{on}; otherwise, work as if @option{sync}=@var{off} +@end table + @item -object memory-backend-ram,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave} Creates a memory backend object, which can be used to back the guest RAM. -- 2.15.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option 2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option Haozhong Zhang @ 2018-01-11 20:06 ` Dr. David Alan Gilbert 2018-01-12 4:13 ` Haozhong Zhang 0 siblings, 1 reply; 5+ messages in thread From: Dr. David Alan Gilbert @ 2018-01-11 20:06 UTC (permalink / raw) To: Haozhong Zhang Cc: qemu-devel, Xiao Guangrong, mst, Stefan Hajnoczi, Paolo Bonzini, Igor Mammedov, Dan Williams, Eduardo Habkost * Haozhong Zhang (haozhong.zhang@intel.com) wrote: > This option controls whether QEMU mmap(2) the memory backend file with > MAP_SYNC flag, which can fully guarantee the guest write persistence > to the backend, if MAP_SYNC flag is supported by the host kernel > (Linux kernel 4.15 and later) and the backend is a file supporting > DAX (e.g., file on ext4/xfs file system mounted with '-o dax'). > > It can take one of following values: > - on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or > 'share=off', QEMU will abort > - off: never pass MAP_SYNC to mmap(2) > - auto (default): if MAP_SYNC is supported and 'share=on', work as if > 'sync=on'; otherwise, work as if 'sync=off' I don't know anything about MAP_SYNC but see two minor comments below: > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > Sugguested-by: Eduardo Habkost <ehabkost@redhat.com> ^ typo! > --- > backends/hostmem-file.c | 39 ++++++++++++++++++++++++++++++++++++++- > docs/nvdimm.txt | 15 ++++++++++++++- > exec.c | 13 ++++++++----- > include/exec/memory.h | 4 ++++ > include/exec/ram_addr.h | 6 +++--- > memory.c | 6 ++++-- > numa.c | 2 +- > qemu-options.hx | 21 ++++++++++++++++++++- > 8 files changed, 92 insertions(+), 14 deletions(-) > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index e319ec1ad8..52fdc0a48a 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -15,6 +15,7 @@ > #include "sysemu/hostmem.h" > #include "sysemu/sysemu.h" > #include "qom/object_interfaces.h" > +#include "qapi-visit.h" > > /* hostmem-file.c */ > /** > @@ -35,6 +36,7 @@ struct HostMemoryBackendFile { > bool discard_data; > char *mem_path; > uint64_t align; > + OnOffAuto sync; > }; > > static void > @@ -60,7 +62,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > path, > backend->size, fb->align, fb->share, > - fb->mem_path, errp); > + fb->sync, fb->mem_path, errp); > g_free(path); > } > #endif > @@ -150,6 +152,38 @@ static void file_memory_backend_set_align(Object *o, Visitor *v, > error_propagate(errp, local_err); > } > > +static void file_memory_backend_get_sync( > + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) > +{ > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); > + OnOffAuto value = fb->sync; > + > + visit_type_OnOffAuto(v, name, &value, errp); > +} > + > +static void file_memory_backend_set_sync( > + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) > +{ > + HostMemoryBackend *backend = MEMORY_BACKEND(obj); > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); > + Error *local_err = NULL; > + OnOffAuto value; > + > + if (host_memory_backend_mr_inited(backend)) { > + error_setg(&local_err, "cannot change property value"); Please make this error clearer; if I was helping someone and saw this error message in a log, I wouldn't know what type of property it was on what device. Please at least include something saying it's memory backend related and sync related; even just including a %s and __func__ would help; even better if you include the name of the object so if you had multiple backend files and one failed you could see which one. Dave > + goto out; > + } > + > + visit_type_OnOffAuto(v, name, &value, &local_err); > + if (local_err) { > + goto out; > + } > + fb->sync = value; > + > + out: > + error_propagate(errp, local_err); > +} > + > static void file_backend_unparent(Object *obj) > { > HostMemoryBackend *backend = MEMORY_BACKEND(obj); > @@ -184,6 +218,9 @@ file_backend_class_init(ObjectClass *oc, void *data) > file_memory_backend_get_align, > file_memory_backend_set_align, > NULL, NULL, &error_abort); > + object_class_property_add(oc, "sync", "OnOffAuto", > + file_memory_backend_get_sync, file_memory_backend_set_sync, > + NULL, NULL, &error_abort); > } > > static void file_backend_instance_finalize(Object *o) > diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt > index e903d8bb09..49b174fe66 100644 > --- a/docs/nvdimm.txt > +++ b/docs/nvdimm.txt > @@ -143,10 +143,23 @@ Guest Data Persistence > ---------------------- > > Though QEMU supports multiple types of vNVDIMM backends on Linux, > -currently the only one that can guarantee the guest write persistence > +if MAP_SYNC is not supported by the host kernel and the backends, > +the only backend that can guarantee the guest write persistence > is the device DAX on the real NVDIMM device (e.g., /dev/dax0.0), to > which all guest access do not involve any host-side kernel cache. > > +mmap(2) flag MAP_SYNC is added since Linux kernel 4.15. On such > +systems, QEMU can mmap(2) the backend with MAP_SYNC, which can > +guarantee the guest write persistence to vNVDIMM. Besides the host > +kernel support, enabling MAP_SYNC in QEMU also requires: > + > + - the backend is a file supporting DAX, e.g., a file on an ext4 or > + xfs file system mounted with "-o dax", > + > + - 'sync' option of memory-backend-file is not 'off', and > + > + - 'share' option of memory-backend-file is 'on'. > + > When using other types of backends, it's suggested to set 'unarmed' > option of '-device nvdimm' to 'on', which sets the unarmed flag of the > guest NVDIMM region mapping structure. This unarmed flag indicates > diff --git a/exec.c b/exec.c > index f4254cb6d3..ce13f8cb21 100644 > --- a/exec.c > +++ b/exec.c > @@ -1600,6 +1600,7 @@ static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > int fd, > bool truncate, > + OnOffAuto sync, > Error **errp) > { > void *area; > @@ -1646,7 +1647,7 @@ static void *file_ram_alloc(RAMBlock *block, > } > > area = qemu_ram_mmap(fd, memory, block->mr->align, > - block->flags & RAM_SHARED, ON_OFF_AUTO_OFF); > + block->flags & RAM_SHARED, sync); > if (area == MAP_FAILED) { > error_setg_errno(errp, errno, > "unable to map backing store for guest RAM"); > @@ -1974,7 +1975,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) > > #ifdef __linux__ > RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > - bool share, int fd, > + bool share, OnOffAuto sync, int fd, > Error **errp) > { > RAMBlock *new_block; > @@ -2017,7 +2018,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > new_block->used_length = size; > new_block->max_length = size; > new_block->flags = share ? RAM_SHARED : 0; > - new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp); > + new_block->host = file_ram_alloc(new_block, size, fd, !file_size, sync, > + errp); > if (!new_block->host) { > g_free(new_block); > return NULL; > @@ -2035,7 +2037,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > > > RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > - bool share, const char *mem_path, > + bool share, OnOffAuto sync, > + const char *mem_path, > Error **errp) > { > int fd; > @@ -2047,7 +2050,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > return NULL; > } > > - block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp); > + block = qemu_ram_alloc_from_fd(size, mr, share, sync, fd, errp); > if (!block) { > if (created) { > unlink(mem_path); > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 07c5d6d597..ff3cd583e9 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -468,6 +468,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, > * @align: alignment of the region base address; if 0, the default alignment > * (getpagesize()) will be used. > * @share: %true if memory must be mmaped with the MAP_SHARED flag > + * @sync: %ON_OFF_AUTO_ON if memory must be mapped with MAP_SYNC flag; > + * %ON_OFF_AUTO_OFF if memory cannot be mapped with MAP_SYNC flag; > + * %ON_OFF_AUTO_AUTO directs QEMU to mmap with MAP_SYNC flag if possible > * @path: the path in which to allocate the RAM. > * @errp: pointer to Error*, to store an error if it happens. > * > @@ -480,6 +483,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > uint64_t size, > uint64_t align, > bool share, > + OnOffAuto sync, > const char *path, > Error **errp); > > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h > index 6cbc02aa0f..4494ae9132 100644 > --- a/include/exec/ram_addr.h > +++ b/include/exec/ram_addr.h > @@ -73,10 +73,10 @@ static inline unsigned long int ramblock_recv_bitmap_offset(void *host_addr, > long qemu_getrampagesize(void); > unsigned long last_ram_page(void); > RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > - bool share, const char *mem_path, > - Error **errp); > + bool share, OnOffAuto sync, > + const char *mem_path, Error **errp); > RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, > - bool share, int fd, > + bool share, OnOffAuto sync, int fd, > Error **errp); > RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, > MemoryRegion *mr, Error **errp); > diff --git a/memory.c b/memory.c > index 449a1429b9..e22f51394e 100644 > --- a/memory.c > +++ b/memory.c > @@ -1572,6 +1572,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > uint64_t size, > uint64_t align, > bool share, > + OnOffAuto sync, > const char *path, > Error **errp) > { > @@ -1580,7 +1581,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > mr->terminates = true; > mr->destructor = memory_region_destructor_ram; > mr->align = align; > - mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp); > + mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, sync, path, errp); > mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; > } > > @@ -1596,7 +1597,8 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr, > mr->ram = true; > mr->terminates = true; > mr->destructor = memory_region_destructor_ram; > - mr->ram_block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp); > + mr->ram_block = qemu_ram_alloc_from_fd(size, mr, share, ON_OFF_AUTO_OFF, fd, > + errp); > mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; > } > #endif > diff --git a/numa.c b/numa.c > index 83675a03f3..93180510a4 100644 > --- a/numa.c > +++ b/numa.c > @@ -457,7 +457,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner, > #ifdef __linux__ > Error *err = NULL; > memory_region_init_ram_from_file(mr, owner, name, ram_size, 0, false, > - mem_path, &err); > + ON_OFF_AUTO_OFF, mem_path, &err); > if (err) { > error_report_err(err); > if (mem_prealloc) { > diff --git a/qemu-options.hx b/qemu-options.hx > index 5ff741a4af..3ee423e6a8 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3974,7 +3974,7 @@ property must be set. These objects are placed in the > > @table @option > > -@item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},align=@var{align} > +@item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},discard-data=@var{on|off},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},align=@var{align},sync=@var{on|off|auto} > > Creates a memory file backend object, which can be used to back > the guest RAM with huge pages. > @@ -4034,6 +4034,25 @@ requires an alignment different than the default one used by QEMU, eg > the device DAX /dev/dax0.0 requires 2M alignment rather than 4K. In > such cases, users can specify the required alignment via this option. > > +The @option{sync} option specifies whether QEMU mmap(2) @option{mem-path} > +with MAP_SYNC flag, which can fully guarantee the guest write > +persistence to @option{mem-path}. MAP_SYNC requires supports from both > +the host kernel (since Linux kernel 4.15) and @option{mem-path} (only > +files supporting DAX). It can take one of following values: > + > +@table @option > +@item @var{on} > +try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or > +@option{share}=@var{off}, QEMU will abort > + > +@item @var{off} > +never pass MAP_SYNC to mmap(2) > + > +@item @var{auto} (default) > +if MAP_SYNC is supported and @option{share}=@var{on}, work as if > +@option{sync}=@var{on}; otherwise, work as if @option{sync}=@var{off} > +@end table > + > @item -object memory-backend-ram,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave} > > Creates a memory backend object, which can be used to back the guest RAM. > -- > 2.15.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option 2018-01-11 20:06 ` Dr. David Alan Gilbert @ 2018-01-12 4:13 ` Haozhong Zhang 0 siblings, 0 replies; 5+ messages in thread From: Haozhong Zhang @ 2018-01-12 4:13 UTC (permalink / raw) To: Dr. David Alan Gilbert Cc: qemu-devel, Xiao Guangrong, mst, Stefan Hajnoczi, Paolo Bonzini, Igor Mammedov, Dan Williams, Eduardo Habkost On 01/11/18 20:06 +0000, Dr. David Alan Gilbert wrote: > * Haozhong Zhang (haozhong.zhang@intel.com) wrote: > > This option controls whether QEMU mmap(2) the memory backend file with > > MAP_SYNC flag, which can fully guarantee the guest write persistence > > to the backend, if MAP_SYNC flag is supported by the host kernel > > (Linux kernel 4.15 and later) and the backend is a file supporting > > DAX (e.g., file on ext4/xfs file system mounted with '-o dax'). > > > > It can take one of following values: > > - on: try to pass MAP_SYNC to mmap(2); if MAP_SYNC is not supported or > > 'share=off', QEMU will abort > > - off: never pass MAP_SYNC to mmap(2) > > - auto (default): if MAP_SYNC is supported and 'share=on', work as if > > 'sync=on'; otherwise, work as if 'sync=off' > > I don't know anything about MAP_SYNC but see two minor comments below: > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > Sugguested-by: Eduardo Habkost <ehabkost@redhat.com> > ^ typo! > [..] > > +static void file_memory_backend_set_sync( > > + Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) > > +{ > > + HostMemoryBackend *backend = MEMORY_BACKEND(obj); > > + HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(obj); > > + Error *local_err = NULL; > > + OnOffAuto value; > > + > > + if (host_memory_backend_mr_inited(backend)) { > > + error_setg(&local_err, "cannot change property value"); > > Please make this error clearer; if I was helping someone and saw > this error message in a log, I wouldn't know what type of property it > was on what device. Please at least include something saying it's > memory backend related and sync related; even just including a %s and > __func__ would help; even better if you include the name of the object > so if you had multiple backend files and one failed you could see which > one. I'll fix the typo, and include the backend id and the property name in the error messages in the next version. Thanks, Haozhong ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-12 4:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-11 14:22 [Qemu-devel] [PATCH v2 0/2] nvdimm: support MAP_SYNC for memory-backend-file Haozhong Zhang 2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 1/2] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() Haozhong Zhang 2018-01-11 14:22 ` [Qemu-devel] [PATCH v2 2/2] hostmem-file: add 'sync' option Haozhong Zhang 2018-01-11 20:06 ` Dr. David Alan Gilbert 2018-01-12 4:13 ` Haozhong Zhang
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).