nvdimm: support MAP_SYNC for memory-backend-file

PATCH In review Zhang Yi 3 participants 15 messages Nov 20, 2018 – Dec 4, 2018 Arm STM32

Share & tools

Zhang Yi

author

2018-11-20 07:49 UTC #0 lore raw

cover nvdimm: support MAP_SYNC for memory-backend-file

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').

A description of MAP_SYNC and MAP_SHARED_VALIDATE can be found at

    https://patchwork.kernel.org/patch/10028151/

In order to make sure that the file metadata is in sync after a fault while we are writing a shared DAX supporting backend files, this patch-set enables QEMU to use MAP_SYNC flag for memory-backend-dax-file.

As the DAX vs DMA truncated issue was solved, we refined the code and send out this feature for the v5 version.

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'

Zhang Yi (7):
numa: Fixed the memory leak of numa error message util/mmap-alloc: switch qemu_ram_mmap() to 'flags' parameter
exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() util/mmap-alloc: Switch the RAM_SYNC flags to OnOffAuto
hostmem: add more information in error messages
hostmem-file: add 'sync' option

 backends/hostmem-file.c               | 45 +++++++++++++++++++++++++++++++++--
 backends/hostmem.c                    |  8 ++++---
 docs/nvdimm.txt                       | 20 +++++++++++++++-
 exec.c                                |  9 +++----
 include/exec/memory.h                 | 18 ++++++++++++++
 include/exec/ram_addr.h               |  1 +
 include/qemu/mmap-alloc.h             | 20 +++++++++++++++-
 include/standard-headers/linux/mman.h | 44 ++++++++++++++++++++++++++++++++++
 numa.c                                |  1 +
 qemu-options.hx                       | 22 ++++++++++++++++-
 util/mmap-alloc.c                     | 26 ++++++++++++++++----
 util/oslib-posix.c                    |  4 +++-
 12 files changed, 200 insertions(+), 18 deletions(-)
 create mode 100644 include/standard-headers/linux/mman.h

Signature · 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:48 UTC #1 lore raw

PATCH 1/7 numa: Fixed the memory leak of numa error message

object_get_canonical_path_component() returns a string which must be freed using g_free().

Signed-off-by Zhang Yi <[email protected]>

---
 numa.c | 1 +
 1 file changed, 1 insertion(+)

numa.c +1 -0

index 50ec016..3875e1e 100644
@@ -533,6 +533,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
             error_report("memory backend %s is used multiple times. Each "
                          "-numa option must use a different memdev value.",
                          path);
+            g_free(path);
             exit(1);
         }

Signature · 1 line

2.7.4

Igor Mammedov

Reviewed-by on v1 1/7

2018-11-26 13:10 UTC #2 lore raw

Re: [Qemu-devel] [PATCH V5_resend 1/7] numa: Fixed the memory leak of numa error message

Quoted 4 lines · On Tue, 20 Nov 2018 15:48:06 +0800 Zhang Yi <[email protected]> wrote:

object_get_canonical_path_component() returns a string which must be freed using g_free().
Signed-off-by Zhang Yi <[email protected]>

Reviewed-by Igor Mammedov <[email protected]>

Quoted 16 lines

---
 numa.c | 1 +
 1 file changed, 1 insertion(+)

numa.c +1 -0

index 50ec016..3875e1e 100644
@@ -533,6 +533,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
             error_report("memory backend %s is used multiple times. Each "
                          "-numa option must use a different memdev value.",
                          path);
+            g_free(path);
             exit(1);
         }

Pankaj Gupta

Reviewed-by on v1 1/7

2018-11-29 08:41 UTC #3 lore raw

Re: [Qemu-devel] [PATCH V5_resend 1/7] numa: Fixed the memory leak of numa error message

In reply to Zhang Yi

Quoted 15 lines

object_get_canonical_path_component() returns a string which must be freed using g_free().
Signed-off-by Zhang Yi <[email protected]>
---
 numa.c | 1 +
 1 file changed, 1 insertion(+)
numa.c +0 -0
index 50ec016..3875e1e 100644

numa.c void memory_region_allocate_system_memory(MemoryRegion

Quoted 9 lines

numa.c +1 -1

@@ -533,6 +533,7 @@ void memory_region_allocate_system_memory(MemoryRegion
 *mr, Object *owner,
             error_report("memory backend %s is used multiple times. Each "
                          "-numa option must use a different memdev value.",
                          path);
+            g_free(path);
             exit(1);
         }
 
--

Reviewed-by Pankaj gupta <[email protected]>

Quoted 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:48 UTC #4 lore raw

PATCH 2/7 util/mmap-alloc: switch qemu_ram_mmap() to 'flags' parameter

In reply to Zhang Yi

As more flag parameters besides the existing 'shared' are going to be added to qemu_ram_mmap(), let's switch 'shared' to a 'flags' parameter in advance, so as to ease the further additions.

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 exec.c                    |  3 +--
 include/qemu/mmap-alloc.h | 19 ++++++++++++++++++-
 util/mmap-alloc.c         |  8 +++++---
 util/oslib-posix.c        |  4 +++-
 4 files changed, 27 insertions(+), 7 deletions(-)

exec.c +1 -2

index bb6170d..273f668 100644
@@ -1859,8 +1859,7 @@ static void *file_ram_alloc(RAMBlock *block,
         perror("ftruncate");
     }
 
-    area = qemu_ram_mmap(fd, memory, block->mr->align,
-                         block->flags & RAM_SHARED);
+    area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
                          "unable to map backing store for guest RAM");

include/qemu/mmap-alloc.h +18 -1

index 50385e3..6fe6ed4 100644
@@ -7,7 +7,24 @@ 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);
+/**
+ * qemu_ram_mmap: mmap the specified file or device.
+ *
+ * Parameters:
+ *  @fd: the file or the device to mmap
+ *  @size: the number of bytes to be mmaped
+ *  @align: if not zero, specify the alignment of the starting mapping address;
+ *          otherwise, the alignment in use will be determined by QEMU.
+ *  @flags: specifies additional properties of the mapping, which can be one or
+ *          bit-or of following values
+ *          - RAM_SHARED: mmap with MAP_SHARED flag
+ *          Other bits are ignored.
+ *
+ * Return:
+ *  On success, return a pointer to the mapped area.
+ *  On failure, return MAP_FAILED.
+ */
+void *qemu_ram_mmap(int fd, size_t size, size_t align, uint32_t flags);
 
 void qemu_ram_munmap(void *ptr, size_t size);

util/mmap-alloc.c +5 -3

index fd329ec..8f0a740 100644
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
+#include "exec/memory.h"
 
 #define HUGETLBFS_MAGIC       0x958458f6

@@ -75,7 +76,7 @@ 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, uint32_t flags)
 {
     /*
      * Note: this always allocates at least one extra page of virtual address

@@ -92,11 +93,12 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared)
      * anonymous memory is OK.
      */
     int anonfd = fd == -1 || qemu_fd_getpagesize(fd) == getpagesize() ? -1 : fd;
-    int flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
-    void *ptr = mmap(0, total, PROT_NONE, flags | MAP_PRIVATE, anonfd, 0);
+    int mmap_flags = anonfd == -1 ? MAP_ANONYMOUS : MAP_NORESERVE;
+    void *ptr = mmap(0, total, PROT_NONE, mmap_flags | MAP_PRIVATE, anonfd, 0);
 #else
     void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
 #endif
+    bool shared = flags & RAM_SHARED;
     size_t offset;
     void *ptr1;

util/oslib-posix.c +3 -1

index fbd0dc8..c28869d 100644
@@ -203,7 +203,9 @@ void *qemu_memalign(size_t alignment, size_t size)
 void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment, bool shared)
 {
     size_t align = QEMU_VMALLOC_ALIGN;
-    void *ptr = qemu_ram_mmap(-1, size, align, shared);
+    uint32_t flags = 0;
+    flags |= shared;
+    void *ptr = qemu_ram_mmap(-1, size, align, flags);
 
     if (ptr == MAP_FAILED) {
         return NULL;

Signature · 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:48 UTC #5 lore raw

PATCH 3/7 exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter

In reply to Zhang Yi

As more flag parameters besides the existing 'share' are going to be added to qemu_ram_alloc_from_{file,fd}(), let's swith 'share' to a 'flags' parameters in advance, so as to ease the further additions.

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 exec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

exec.c +4 -2

index 273f668..e92a7da 100644
@@ -1810,6 +1810,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             ram_addr_t memory,
                             int fd,
                             bool truncate,
+                            uint32_t flags,
                             Error **errp)
 {
     void *area;

@@ -1859,7 +1860,7 @@ static void *file_ram_alloc(RAMBlock *block,
         perror("ftruncate");
     }
 
-    area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
+    area = qemu_ram_mmap(fd, memory, block->mr->align, flags);
     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
                          "unable to map backing store for guest RAM");

@@ -2278,7 +2279,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 = ram_flags;
-    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,
+            ram_flags, errp);
     if (!new_block->host) {
         g_free(new_block);
         return NULL;

Signature · 1 line

2.7.4

Pankaj Gupta

2018-11-29 09:11 UTC #6 lore raw

Re: [Qemu-devel] [PATCH V5_resend 3/7] exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter

Quoted 25 lines

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 exec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

exec.c +1 -0

index 273f668..e92a7da 100644
@@ -1810,6 +1810,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             ram_addr_t memory,
                             int fd,
                             bool truncate,
+                            uint32_t flags,
                             Error **errp)
 {
     void *area;

exec.c static void *file_ram_alloc(RAMBlock *block, · line 1863

Quoted 5 lines

exec.c +1 -1

@@ -1859,7 +1860,7 @@ static void *file_ram_alloc(RAMBlock *block,
         perror("ftruncate");
     }
 
-    area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
+    area = qemu_ram_mmap(fd, memory, block->mr->align, flags);

I think this change can be squashed with patch2 unless I am not missing anything here.

Quoted 19 lines

exec.c +2 -1

     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
                          "unable to map backing store for guest RAM");

@@ -2278,7 +2279,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 = ram_flags;
-    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,
+            ram_flags, errp);
     if (!new_block->host) {
         g_free(new_block);

         return NULL;

--
2.7.4

Yi Zhang

author

2018-12-04 07:37 UTC #7 lore raw

Re: [Qemu-devel] [PATCH V5_resend 3/7] exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter

Quoted 28 lines · Pankaj Gupta

Quoted 28 lines

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 exec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

exec.c +1 -0

index 273f668..e92a7da 100644
@@ -1810,6 +1810,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             ram_addr_t memory,
                             int fd,
                             bool truncate,
+                            uint32_t flags,
                             Error **errp)
 {
     void *area;

@@ -1859,7 +1860,7 @@ static void *file_ram_alloc(RAMBlock *block,
         perror("ftruncate");
     }

Quoted 5 lines

exec.c static void *file_ram_alloc(RAMBlock *block, · line 1863
Quoted 2 lines
exec.c +1 -1
-    area = qemu_ram_mmap(fd, memory, block->mr->align, block->flags);
+    area = qemu_ram_mmap(fd, memory, block->mr->align, flags);
I think this change can be squashed with patch2 unless I am not missing anything here.

Yes, right, nothing just split the diffrent function in diffrent patch. It's OK to squashed these 2 together.
Thanks for the review.

Yi.

Quoted 20 lines

exec.c +2 -1

     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
                          "unable to map backing store for guest RAM");

@@ -2278,7 +2279,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 = ram_flags;
-    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,
+            ram_flags, errp);
     if (!new_block->host) {
         g_free(new_block);

         return NULL;

--
2.7.4

Zhang Yi

author

2018-11-20 07:49 UTC #8 lore raw

PATCH 4/7 util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()

In reply to Zhang Yi

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 set of RAM_SYNC flags are added to qemu_ram_mmap():

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 exec.c                                |  2 +-
 include/exec/memory.h                 |  3 +++
 include/exec/ram_addr.h               |  1 +
 include/qemu/mmap-alloc.h             |  1 +
 include/standard-headers/linux/mman.h | 44 +++++++++++++++++++++++++++++++++++
 util/mmap-alloc.c                     | 14 +++++++----
 6 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 include/standard-headers/linux/mman.h

exec.c +1 -1

index e92a7da..dc4d180 100644
@@ -2241,7 +2241,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
     int64_t file_size;
 
     /* Just support these ram flags by now. */
-    assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
+    assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_SYNC)) == 0);
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");

include/exec/memory.h +3 -0

index 667466b..33a4e2c 100644
@@ -126,6 +126,9 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 /* RAM is a persistent kind memory */
 #define RAM_PMEM (1 << 5)
 
+/* RAM can be mmap by a MAP_SYNC flag */
+#define RAM_SYNC (1 << 6)
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
                                        IOMMUNotifierFlag flags,
                                        hwaddr start, hwaddr end,

include/exec/ram_addr.h +1 -0

index 9ecd911..d239ce7 100644
@@ -87,6 +87,7 @@ long qemu_getrampagesize(void);
  *              or bit-or of following values
  *              - RAM_SHARED: mmap the backing file or device with MAP_SHARED
  *              - RAM_PMEM: the backend @mem_path or @fd is persistent memory
+ *              - RAM_SYNC:   mmap with MAP_SYNC flag
  *              Other bits are ignored.
  *  @mem_path or @fd: specify the backing file or device
  *  @errp: pointer to Error*, to store an error if it happens

include/qemu/mmap-alloc.h +1 -0

index 6fe6ed4..1755a8b 100644
@@ -18,6 +18,7 @@ size_t qemu_mempath_getpagesize(const char *mem_path);
  *  @flags: specifies additional properties of the mapping, which can be one or
  *          bit-or of following values
  *          - RAM_SHARED: mmap with MAP_SHARED flag
+ *          - RAM_SYNC:   mmap with MAP_SYNC flag
  *          Other bits are ignored.
  *
  * Return:

include/standard-headers/linux/mman.h new file +44 -0

44 lines not on this page · raw message

util/mmap-alloc.c +10 -4

index 8f0a740..f411df7 100644
@@ -14,6 +14,7 @@
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
 #include "exec/memory.h"
+#include "standard-headers/linux/mman.h"
 
 #define HUGETLBFS_MAGIC       0x958458f6

@@ -99,6 +100,7 @@ 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;
+    int mmap_xflags = 0;
     size_t offset;
     void *ptr1;

@@ -109,16 +111,20 @@ 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 ((flags & RAM_SYNC) && shared) {
+        mmap_xflags |= MAP_SYNC_FLAGS;
+    }
 
     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) {
-        munmap(ptr, total);
-        return MAP_FAILED;
+    if ((ptr1 == MAP_FAILED) && (mmap_xflags & MAP_SYNC_FLAGS)) {
+        mmap_xflags &= ~MAP_SYNC_FLAGS;
+        goto retry_mmap_fd;
     }
 
     if (offset > 0) {

Signature · 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:49 UTC #9 lore raw

PATCH 5/7 util/mmap-alloc: Switch the RAM_SYNC flags to OnOffAuto

In reply to Zhang Yi

Signed-off-by Zhang Yi <[email protected]>

A set of RAM_SYNC_ON_OFF_AUTO{AUTO,ON,OFF} flags are added to qemu_ram_mmap():

- If RAM_SYNC_ON_OFF_AUTO_ON is present, 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 RAM_SYNC_ON_OFF_AUTO_OFF is present, qemu_ram_mmap() will never pass MAP_SYNC to mmap().

- If RAM_SYNC_ON_OFF_AUTO_AUTO is present, 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 RAM_SYNC_ON_OFF_AUTO_ON is present;
* otherwise, qemu_ram_mmap() will work as if RAM_SYNC_ON_OFF_AUTO_OFF is present.

---
 include/exec/memory.h |  9 ++++++++-
 util/mmap-alloc.c     | 12 ++++++++++--
 2 files changed, 18 insertions(+), 3 deletions(-)

include/exec/memory.h +8 -1

index 33a4e2c..c74c467 100644
@@ -127,7 +127,14 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 #define RAM_PMEM (1 << 5)
 
 /* RAM can be mmap by a MAP_SYNC flag */
-#define RAM_SYNC (1 << 6)
+#define RAM_SYNC_SHIFT  6
+#define RAM_SYNC_SHIFT_AUTO  7
+
+#define RAM_SYNC_ON_OFF_AUTO_ON   (1UL << RAM_SYNC_SHIFT)
+#define RAM_SYNC_ON_OFF_AUTO_OFF  (0UL << RAM_SYNC_SHIFT)
+#define RAM_SYNC_ON_OFF_AUTO_AUTO (1UL << RAM_SYNC_SHIFT_AUTO)
+
+#define RAM_SYNC (RAM_SYNC_ON_OFF_AUTO_ON | RAM_SYNC_ON_OFF_AUTO_AUTO)
 
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
                                        IOMMUNotifierFlag flags,

util/mmap-alloc.c +10 -2

index f411df7..fe9303f 100644
@@ -111,6 +111,10 @@ 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 ((flags & RAM_SYNC_ON_OFF_AUTO_ON) &&
+        (!shared || !MAP_SYNC_FLAGS)) {
+        return MAP_FAILED;
+    }
     if ((flags & RAM_SYNC) && shared) {
         mmap_xflags |= MAP_SYNC_FLAGS;
     }

@@ -123,8 +127,12 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, uint32_t flags)
                 (shared ? MAP_SHARED : MAP_PRIVATE) | mmap_xflags,
                 fd, 0);
     if ((ptr1 == MAP_FAILED) && (mmap_xflags & MAP_SYNC_FLAGS)) {
-        mmap_xflags &= ~MAP_SYNC_FLAGS;
-        goto retry_mmap_fd;
+        if (flags & RAM_SYNC_ON_OFF_AUTO_AUTO) {
+            mmap_xflags &= ~MAP_SYNC_FLAGS;
+            goto retry_mmap_fd;
+        }
+        munmap(ptr, total);
+        return MAP_FAILED;
     }
 
     if (offset > 0) {

Signature · 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:49 UTC #10 lore raw

PATCH 6/7 hostmem: add more information in error messages

In reply to Zhang Yi

When there are multiple memory backends in use, including the object type name, ID and the property name in the error message can help users to locate the error.

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 backends/hostmem-file.c | 6 ++++--
 backends/hostmem.c      | 8 +++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

backends/hostmem-file.c +4 -2

index e640749..0dd7a90 100644
@@ -82,7 +82,8 @@ static void set_mem_path(Object *o, const char *str, Error **errp)
     HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
 
     if (host_memory_backend_mr_inited(backend)) {
-        error_setg(errp, "cannot change property value");
+        error_setg(errp, "cannot change property 'mem-path' of %s",
+                   object_get_typename(o));
         return;
     }
     g_free(fb->mem_path);

@@ -120,7 +121,8 @@ static void file_memory_backend_set_align(Object *o, Visitor *v,
     uint64_t val;
 
     if (host_memory_backend_mr_inited(backend)) {
-        error_setg(&local_err, "cannot change property value");
+        error_setg(&local_err, "cannot change property '%s' of %s",
+                   name, object_get_typename(o));
         goto out;
     }

backends/hostmem.c +5 -3

index 1a89342..e2bcf9f 100644
@@ -47,7 +47,8 @@ host_memory_backend_set_size(Object *obj, Visitor *v, const char *name,
     uint64_t value;
 
     if (host_memory_backend_mr_inited(backend)) {
-        error_setg(&local_err, "cannot change property value");
+        error_setg(&local_err, "cannot change property %s of %s ",
+                   name, object_get_typename(obj));
         goto out;
     }

@@ -56,8 +57,9 @@ host_memory_backend_set_size(Object *obj, Visitor *v, const char *name,
         goto out;
     }
     if (!value) {
-        error_setg(&local_err, "Property '%s.%s' doesn't take value '%"
-                   PRIu64 "'", object_get_typename(obj), name, value);
+        error_setg(&local_err,
+                   "property '%s' of %s doesn't take value '%" PRIu64 "'",
+                   name, object_get_typename(obj), value);
         goto out;
     }
     backend->size = value;

Signature · 1 line

2.7.4

Zhang Yi

author

2018-11-20 07:49 UTC #11 lore raw

PATCH 7/7 hostmem-file: add 'sync' option

In reply to Zhang Yi

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 <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 backends/hostmem-file.c | 39 +++++++++++++++++++++++++++++++++++++++
 docs/nvdimm.txt         | 20 +++++++++++++++++++-
 include/exec/memory.h   |  8 ++++++++
 qemu-options.hx         | 22 +++++++++++++++++++++-
 4 files changed, 87 insertions(+), 2 deletions(-)

backends/hostmem-file.c +39 -0

index 0dd7a90..73cf181 100644
@@ -16,6 +16,7 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/sysemu.h"
 #include "qom/object_interfaces.h"
+#include "qapi/qapi-visit.h"
 
 /* hostmem-file.c */
 /**

@@ -36,6 +37,7 @@ struct HostMemoryBackendFile {
     uint64_t align;
     bool discard_data;
     bool is_pmem;
+    OnOffAuto sync;
 };
 
 static void

@@ -62,6 +64,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
                                  path,
                                  backend->size, fb->align,
                                  (backend->share ? RAM_SHARED : 0) |
+                                 qemu_ram_sync_flags(fb->sync) |
                                  (fb->is_pmem ? RAM_PMEM : 0),
                                  fb->mem_path, errp);
         g_free(path);

@@ -136,6 +139,39 @@ 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 '%s' of %s",
+                   name, object_get_typename(obj));
+        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 bool file_memory_backend_get_pmem(Object *o, Error **errp)
 {
     return MEMORY_BACKEND_FILE(o)->is_pmem;

@@ -203,6 +239,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
     object_class_property_add_bool(oc, "pmem",
         file_memory_backend_get_pmem, file_memory_backend_set_pmem,
         &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)

docs/nvdimm.txt +19 -1

index 5f158a6..3d89174 100644
@@ -142,11 +142,29 @@ backend of vNVDIMM:
 Guest Data Persistence
 ----------------------
 
+vNVDIMM is designed and implemented to guarantee the guest data
+persistence on the backends even on the host crash and power
+failures. However, there are still some requirements and limitations
+as explained below.
+
 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

include/exec/memory.h +8 -0

index c74c467..b398abb 100644
@@ -26,6 +26,7 @@
 #include "qom/object.h"
 #include "qemu/rcu.h"
 #include "hw/qdev-core.h"
+#include "qapi/error.h"
 
 #define RAM_ADDR_INVALID (~(ram_addr_t)0)

@@ -136,6 +137,13 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 
 #define RAM_SYNC (RAM_SYNC_ON_OFF_AUTO_ON | RAM_SYNC_ON_OFF_AUTO_AUTO)
 
+static inline uint64_t qemu_ram_sync_flags(OnOffAuto v)
+{
+    return v == ON_OFF_AUTO_OFF ? RAM_SYNC_ON_OFF_AUTO_OFF :
+           v == ON_OFF_AUTO_ON ? RAM_SYNC_ON_OFF_AUTO_ON :
+           RAM_SYNC_ON_OFF_AUTO_AUTO;
+}
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
                                        IOMMUNotifierFlag flags,
                                        hwaddr start, hwaddr end,

qemu-options.hx +21 -1

index 08f8516..77e8810 100644
@@ -3928,7 +3928,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.

@@ -4003,6 +4003,26 @@ If @option{pmem} is set to 'on', QEMU will take necessary operations to
 guarantee the persistence of its own writes to @option{mem-path}
 (e.g. in vNVDIMM label emulation and live migration).
 
+The @option{sync} option specifies whether QEMU mmap(2) @option{mem-path}
+with MAP_SYNC flag, which can guarantee the guest write persistence to
+@option{mem-path} even on the host crash and power failures. 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},share=@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.

Signature · 1 line

2.7.4

Pankaj Gupta

2018-11-26 08:46 UTC #12 lore raw

Re: [Qemu-devel] [PATCH V5_resend 7/7] hostmem-file: add 'sync' option

Quoted 4 lines

This option controls whether QEMU mmap(2) the memory backend file with MAP_SYNC flag, which can fully guarantee the guest write persistence

Not sure 'persistence' is the right word here. I think it should be something like 'consistent filesystem metadata for every guest write' just to avoid any confusion.

Quoted 112 lines

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').

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 backends/hostmem-file.c | 39 +++++++++++++++++++++++++++++++++++++++
 docs/nvdimm.txt         | 20 +++++++++++++++++++-
 include/exec/memory.h   |  8 ++++++++
 qemu-options.hx         | 22 +++++++++++++++++++++-
 4 files changed, 87 insertions(+), 2 deletions(-)

backends/hostmem-file.c +3 -0

index 0dd7a90..73cf181 100644
@@ -16,6 +16,7 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/sysemu.h"
 #include "qom/object_interfaces.h"
+#include "qapi/qapi-visit.h"
 
 /* hostmem-file.c */
 /**

@@ -36,6 +37,7 @@ struct HostMemoryBackendFile {
     uint64_t align;
     bool discard_data;
     bool is_pmem;
+    OnOffAuto sync;
 };
 
 static void

@@ -62,6 +64,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
 **errp)
                                  path,
                                  backend->size, fb->align,
                                  (backend->share ? RAM_SHARED : 0) |
+                                 qemu_ram_sync_flags(fb->sync) |
                                  (fb->is_pmem ? RAM_PMEM : 0),
                                  fb->mem_path, errp);

         g_free(path);

backends/hostmem-file.c +33 -0

@@ -136,6 +139,39 @@ 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 '%s' of %s",
+                   name, object_get_typename(obj));
+        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 bool file_memory_backend_get_pmem(Object *o, Error **errp)
 {

     return MEMORY_BACKEND_FILE(o)->is_pmem;

backends/hostmem-file.c +3 -0

@@ -203,6 +239,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
     object_class_property_add_bool(oc, "pmem",
         file_memory_backend_get_pmem, file_memory_backend_set_pmem,
         &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)

docs/nvdimm.txt +4 -0

index 5f158a6..3d89174 100644
@@ -142,11 +142,29 @@ backend of vNVDIMM:
 Guest Data Persistence
 ----------------------
 
+vNVDIMM is designed and implemented to guarantee the guest data
+persistence on the backends even on the host crash and power
+failures. However, there are still some requirements and limitations
+as explained below.

docs/nvdimm.txt backend of vNVDIMM: · line 157

Quoted 10 lines

docs/nvdimm.txt +5 -1

+
 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

I think its important to clear about the type of backend. i.e DAX file on real NVDIMM device backend.

Thanks,
Pankaj

Quoted 88 lines

docs/nvdimm.txt +10 -0

+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

include/exec/memory.h +8 -0

index c74c467..b398abb 100644
@@ -26,6 +26,7 @@
 #include "qom/object.h"
 #include "qemu/rcu.h"
 #include "hw/qdev-core.h"
+#include "qapi/error.h"
 
 #define RAM_ADDR_INVALID (~(ram_addr_t)0)

@@ -136,6 +137,13 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 
 #define RAM_SYNC (RAM_SYNC_ON_OFF_AUTO_ON | RAM_SYNC_ON_OFF_AUTO_AUTO)
 
+static inline uint64_t qemu_ram_sync_flags(OnOffAuto v)
+{
+    return v == ON_OFF_AUTO_OFF ? RAM_SYNC_ON_OFF_AUTO_OFF :
+           v == ON_OFF_AUTO_ON ? RAM_SYNC_ON_OFF_AUTO_ON :
+           RAM_SYNC_ON_OFF_AUTO_AUTO;
+}
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
                                        IOMMUNotifierFlag flags,
                                        hwaddr start, hwaddr end,

qemu-options.hx +1 -1

index 08f8516..77e8810 100644
@@ -3928,7 +3928,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.

qemu-options.hx +0 -0

@@ -4003,6 +4003,26 @@ If @option{pmem} is set to 'on', QEMU will take

necessary operations to
guarantee the persistence of its own writes to @option{mem-path} (e.g. in vNVDIMM label emulation and live migration).

qemu-options.hx +20 -1

+The @option{sync} option specifies whether QEMU mmap(2) @option{mem-path}
+with MAP_SYNC flag, which can guarantee the guest write persistence to
+@option{mem-path} even on the host crash and power failures. 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},share=@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.7.4

Yi Zhang

author

2018-12-04 07:37 UTC #13 lore raw

Re: [Qemu-devel] [PATCH V5_resend 7/7] hostmem-file: add 'sync' option

Quoted 8 lines · Pankaj Gupta

Quoted 4 lines
This option controls whether QEMU mmap(2) the memory backend file with MAP_SYNC flag, which can fully guarantee the guest write persistence
Not sure 'persistence' is the right word here. I think it should be something like 'consistent filesystem metadata for every guest write' just to avoid any confusion.

Yes, Agree, the main porpurse of this flag is "*consistent filesystem metadata", I will improve that comments in next version

Regards
Yi.

Quoted 218 lines

Quoted 113 lines

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').

Signed-off-by Haozhong Zhang <[email protected]>

Signed-off-by Zhang Yi <[email protected]>

---
 backends/hostmem-file.c | 39 +++++++++++++++++++++++++++++++++++++++
 docs/nvdimm.txt         | 20 +++++++++++++++++++-
 include/exec/memory.h   |  8 ++++++++
 qemu-options.hx         | 22 +++++++++++++++++++++-
 4 files changed, 87 insertions(+), 2 deletions(-)

backends/hostmem-file.c +3 -0

index 0dd7a90..73cf181 100644
@@ -16,6 +16,7 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/sysemu.h"
 #include "qom/object_interfaces.h"
+#include "qapi/qapi-visit.h"
 
 /* hostmem-file.c */
 /**

@@ -36,6 +37,7 @@ struct HostMemoryBackendFile {
     uint64_t align;
     bool discard_data;
     bool is_pmem;
+    OnOffAuto sync;
 };
 
 static void

@@ -62,6 +64,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
 **errp)
                                  path,
                                  backend->size, fb->align,
                                  (backend->share ? RAM_SHARED : 0) |
+                                 qemu_ram_sync_flags(fb->sync) |
                                  (fb->is_pmem ? RAM_PMEM : 0),
                                  fb->mem_path, errp);

         g_free(path);

backends/hostmem-file.c +33 -0

@@ -136,6 +139,39 @@ 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 '%s' of %s",
+                   name, object_get_typename(obj));
+        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 bool file_memory_backend_get_pmem(Object *o, Error **errp)
 {

     return MEMORY_BACKEND_FILE(o)->is_pmem;

backends/hostmem-file.c +3 -0

@@ -203,6 +239,9 @@ file_backend_class_init(ObjectClass *oc, void *data)
     object_class_property_add_bool(oc, "pmem",
         file_memory_backend_get_pmem, file_memory_backend_set_pmem,
         &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)

docs/nvdimm.txt +4 -0

index 5f158a6..3d89174 100644
@@ -142,11 +142,29 @@ backend of vNVDIMM:
 Guest Data Persistence
 ----------------------
 
+vNVDIMM is designed and implemented to guarantee the guest data
+persistence on the backends even on the host crash and power
+failures. However, there are still some requirements and limitations
+as explained below.

docs/nvdimm.txt backend of vNVDIMM: · line 157

Quoted 10 lines

docs/nvdimm.txt +5 -1

+
 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

I think its important to clear about the type of backend. i.e DAX file on real NVDIMM device backend.

Thanks,
Pankaj

Quoted 88 lines

docs/nvdimm.txt +10 -0

+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

include/exec/memory.h +8 -0

index c74c467..b398abb 100644
@@ -26,6 +26,7 @@
 #include "qom/object.h"
 #include "qemu/rcu.h"
 #include "hw/qdev-core.h"
+#include "qapi/error.h"
 
 #define RAM_ADDR_INVALID (~(ram_addr_t)0)

@@ -136,6 +137,13 @@ typedef struct IOMMUNotifier IOMMUNotifier;
 
 #define RAM_SYNC (RAM_SYNC_ON_OFF_AUTO_ON | RAM_SYNC_ON_OFF_AUTO_AUTO)
 
+static inline uint64_t qemu_ram_sync_flags(OnOffAuto v)
+{
+    return v == ON_OFF_AUTO_OFF ? RAM_SYNC_ON_OFF_AUTO_OFF :
+           v == ON_OFF_AUTO_ON ? RAM_SYNC_ON_OFF_AUTO_ON :
+           RAM_SYNC_ON_OFF_AUTO_AUTO;
+}
+
 static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
                                        IOMMUNotifierFlag flags,
                                        hwaddr start, hwaddr end,

qemu-options.hx +1 -1

index 08f8516..77e8810 100644
@@ -3928,7 +3928,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.

qemu-options.hx +0 -0

@@ -4003,6 +4003,26 @@ If @option{pmem} is set to 'on', QEMU will take

necessary operations to
guarantee the persistence of its own writes to @option{mem-path} (e.g. in vNVDIMM label emulation and live migration).

qemu-options.hx +20 -1

+The @option{sync} option specifies whether QEMU mmap(2) @option{mem-path}
+with MAP_SYNC flag, which can guarantee the guest write persistence to
+@option{mem-path} even on the host crash and power failures. 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},share=@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.7.4

Yi Zhang

author

2018-11-26 03:01 UTC #14 lore raw

In reply to Zhang Yi

Ping for any review/comments
Thanks
Yi

Quoted 14 lines · Zhang Yi

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').
A description of MAP_SYNC and MAP_SHARED_VALIDATE can be found at
    https://patchwork.kernel.org/patch/10028151/
In order to make sure that the file metadata is in sync after a fault while we are writing a shared DAX supporting backend files, this patch-set enables QEMU to use MAP_SYNC flag for memory-backend-dax-file.
As the DAX vs DMA truncated issue was solved, we refined the code and send out this feature for the v5 version.

Quoted 6 lines

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 v5:
* Add patch 1 to fix a memory leak issue.
* Refine the patch 4-6
* Remove the patch 3 as we already change the parameter from "shared" to "flags"

Changes in v4:
* Add patch 1-3 to switch some functions to a single 'flags' parameters. (Michael S. Tsirkin)
* v3 patch 1-3 become v4 patch 4-6.
* Patch 4: move definitions of MAP_SYNC and MAP_SHARED_VALIDATE to a new header file under include/standard-headers/linux/. (Michael S. Tsirkin)
* Patch 6: refine the description of the 'sync' option. (Michael S. Tsirkin)

Changes in v3:
* Patch 1: add MAP_SHARED_VALIDATE in both sync=on and sync=auto cases, and add back the retry mechanism. MAP_SYNC will be ignored by Linux kernel 4.15 if MAP_SHARED_VALIDATE is missed.
* Patch 1: define MAP_SYNC and MAP_SHARED_VALIDATE as 0 on non-Linux platforms in order to make qemu_ram_mmap() compile on those platforms.
* Patch 2&3: include more information in error messages of memory-backend in hope to help user to identify the error. (Dr. David Alan Gilbert)
* Patch 3: fix typo in the commit message. (Dr. David Alan Gilbert)

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)

Quoted 28 lines

Zhang Yi (7):
numa: Fixed the memory leak of numa error message util/mmap-alloc: switch qemu_ram_mmap() to 'flags' parameter
exec: switch qemu_ram_alloc_from_{file, fd} to the 'flags' parameter util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap() util/mmap-alloc: Switch the RAM_SYNC flags to OnOffAuto
hostmem: add more information in error messages
hostmem-file: add 'sync' option
 backends/hostmem-file.c               | 45 +++++++++++++++++++++++++++++++++--
 backends/hostmem.c                    |  8 ++++---
 docs/nvdimm.txt                       | 20 +++++++++++++++-
 exec.c                                |  9 +++----
 include/exec/memory.h                 | 18 ++++++++++++++
 include/exec/ram_addr.h               |  1 +
 include/qemu/mmap-alloc.h             | 20 +++++++++++++++-
 include/standard-headers/linux/mman.h | 44 ++++++++++++++++++++++++++++++++++
 numa.c                                |  1 +
 qemu-options.hx                       | 22 ++++++++++++++++-
 util/mmap-alloc.c                     | 26 ++++++++++++++++----
 util/oslib-posix.c                    |  4 +++-
 12 files changed, 200 insertions(+), 18 deletions(-)
 create mode 100644 include/standard-headers/linux/mman.h
Signature · 1 line
2.7.4