> > 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?
> The one with the least impact; the migration code is getting more and
> more complex, so having to do the 'if (is_pmem)' check everywhere isn't
> nice, passing an 'ops' pointer in is better. However if you can do the
> 'flush before complete' instead then the amount of code change is a LOT
> smaller.
> The only other question is whether from your pmem view, the
> flush-before-complete causes any problems; in the worst case, how long
> could the flush take?
According to my understanding, flush-before-complete should be OK and save.
Haozhong gave the hint that the flush step by step may be faster and I think the
benchmark shows that they are close.
The worst case is that all the pmem-like memory will be flushed after completing
migration, I am not sure whether there are some unused pmem for new guest will
also
be flushed.
> Dave
________________________________
From: Dr. David Alan Gilbert <[email protected]>
Sent: Thursday, May 31, 2018 2:42:19 PM
To: Junyan He
Cc: Stefan Hajnoczi; [email protected]; Haozhong Zhang;
[email protected]; [email protected]; [email protected];
[email protected]; [email protected]; Junyan He; [email protected];
[email protected]; [email protected]; [email protected]
Subject: Re: [Qemu-devel] [PATCH V5 0/9] nvdimm: guarantee persistence of QEMU
writes to persistent memory
* Junyan He ([email protected]) wrote:
> > 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?
The one with the least impact; the migration code is getting more and
more complex, so having to do the 'if (is_pmem)' check everywhere isn't
nice, passing an 'ops' pointer in is better. However if you can do the
'flush before complete' instead then the amount of code change is a LOT
smaller.
The only other question is whether from your pmem view, the
flush-before-complete causes any problems; in the worst case, how long
could the flush take?
Dave
> ________________________________
> From: Stefan Hajnoczi <[email protected]>
> Sent: Thursday, May 31, 2018 1:18:58 PM
> To: [email protected]
> Cc: [email protected]; Haozhong Zhang; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Junyan He; [email protected];
> [email protected]; [email protected]; [email protected]
> 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
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
