Re: [Qemu-devel] [PATCH V5 0/9] nvdimm: guarantee persistence of QEMU writes to persistent memory

[Junyan He]

> Also, there was a discussion about leaving the code unchanged but adding
> an nvdimm_flush() call at the very end of migration.  I think someone
> benchmarked it but can't find the email.  Please post a link or
> summarize the results, because that approach would be much less
> invasive.  Thanks!


And previous comments:


> > > 2. The migration/ram code is invasive.  Is it really necessary to
> > >    persist data each time pages are loaded from a migration stream?  It
> > >    seems simpler to migrate as normal and call pmem_persist() just once
> > >    after RAM has been migrated but before the migration completes.
> >
> > The concern is about the overhead of cache flush.
> >
> > In this patch series, if possible, QEMU will use pmem_mem{set,cpy}_nodrain
> > APIs to copy NVDIMM blocks. Those APIs use movnt (if it's available) and
> > can avoid the subsequent cache flush.
> >
> > Anyway, I'll make some microbenchmark to check which one will be better.

> The problem is not just the overhead; the problem is the code
> complexity; this series makes all the paths through the migration code
> more complex in places we wouldn't expect to change.

I already use the migration info tool and list the result in the Mail just 
after this patch set sent:

Disable all haozhong's pmem_drain and  pmem_memset_nodrain  kind function call
and make the cleanup function do the flush job like this:

static int ram_load_cleanup(void *opaque)
{
    RAMBlock *rb;
    RAMBLOCK_FOREACH(rb) {
        if (ramblock_is_pmem(rb)) {
            pmem_persist(rb->host, rb->used_length);
         }
    }

    xbzrle_load_cleanup();
    compress_threads_load_cleanup();

    RAMBLOCK_FOREACH(rb) {
        g_free(rb->receivedmap);
        rb->receivedmap = NULL;
    }
    return 0;
}


The migrate info result is:

Haozhong's Manner

(qemu) migrate -d tcp:localhost:4444
(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off 
compress: off events: off postcopy-ram: off x-colo: off release-ram: off block: 
off return-path: off pause-before-switchover: off x-multifd: off dirty-bitmaps: 
off postcopy-blocktime: off
Migration status: completed
total time: 333668 milliseconds
downtime: 17 milliseconds
setup: 50 milliseconds
transferred ram: 10938039 kbytes
throughput: 268.55 mbps
remaining ram: 0 kbytes
total ram: 11027272 kbytes
duplicate: 35533 pages
skipped: 0 pages
normal: 2729095 pages
normal bytes: 10916380 kbytes
dirty sync count: 4
page size: 4 kbytes
(qemu)


flush before complete

QEMU 2.12.50 monitor - type 'help' for more information
(qemu) migrate -d tcp:localhost:4444
(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off 
compress: off events: off postcopy-ram: off x-colo: off release-ram: off block: 
off return-path: off pause-before-switchover: off x-multifd: off dirty-bitmaps: 
off postcopy-blocktime: off
Migration status: completed
total time: 334836 milliseconds
downtime: 17 milliseconds
setup: 49 milliseconds
transferred ram: 10978886 kbytes
throughput: 268.62 mbps
remaining ram: 0 kbytes
total ram: 11027272 kbytes
duplicate: 23149 pages
skipped: 0 pages
normal: 2739314 pages
normal bytes: 10957256 kbytes
dirty sync count: 4
page size: 4 kbytes
(qemu)


So Haozhong's manner seems to be a little faster and I choose to keep that.

If you want to choose this manner, the code will be clean and no need for

>   typedef struct {
>       void (*memset)(void *s, int c, size_t n);
>       void (*memcpy)(void *dest, const void *src, size_t n);
>   } MemoryOperations;


performance is close, and I am a little new in Qemu:), so both options are OK 
for me,

Which one do you prefer to?

________________________________
From: Stefan Hajnoczi <address@hidden>
Sent: Thursday, May 31, 2018 1:18:58 PM
To: address@hidden
Cc: address@hidden; Haozhong Zhang; address@hidden; address@hidden; 
address@hidden; address@hidden; address@hidden; address@hidden; Junyan He; 
address@hidden; address@hidden; address@hidden; address@hidden
Subject: Re: [Qemu-devel] [PATCH V5 0/9] nvdimm: guarantee persistence of QEMU 
writes to persistent memory

David Gilbert previously suggested a memory access interface.  I guess
it would look something like this:

  typedef struct {
      void (*memset)(void *s, int c, size_t n);
      void (*memcpy)(void *dest, const void *src, size_t n);
  } MemoryOperations;

That way code doesn't need if (pmem) A else B.  It can just do
mem_ops->foo().  Have you looked into this idea?

Also, there was a discussion about leaving the code unchanged but adding
an nvdimm_flush() call at the very end of migration.  I think someone
benchmarked it but can't find the email.  Please post a link or
summarize the results, because that approach would be much less
invasive.  Thanks!

Stefan