[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 6/8] migration/ram: ensure write persistence
From: |
Haozhong Zhang |
Subject: |
Re: [Qemu-devel] [PATCH v2 6/8] migration/ram: ensure write persistence on loading normal pages to PMEM |
Date: |
Wed, 7 Feb 2018 20:02:43 +0800 |
User-agent: |
NeoMutt/20171027 |
On 02/07/18 11:49 +0000, Dr. David Alan Gilbert wrote:
> * Haozhong Zhang (address@hidden) wrote:
> > When loading a normal page to persistent memory, load its data by
> > libpmem function pmem_memcpy_nodrain() instead of memcpy(). Combined
> > with a call to pmem_drain() at the end of memory loading, we can
> > guarantee all those normal pages are persistenly loaded to PMEM.
> >
> > Signed-off-by: Haozhong Zhang <address@hidden>
> > ---
> > include/migration/qemu-file-types.h | 1 +
> > include/qemu/pmem.h | 6 ++++++
> > migration/qemu-file.c | 41
> > ++++++++++++++++++++++++++++---------
> > migration/ram.c | 6 +++++-
> > 4 files changed, 43 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/migration/qemu-file-types.h
> > b/include/migration/qemu-file-types.h
> > index bd6d7dd7f9..bb5c547498 100644
> > --- a/include/migration/qemu-file-types.h
> > +++ b/include/migration/qemu-file-types.h
> > @@ -34,6 +34,7 @@ void qemu_put_be16(QEMUFile *f, unsigned int v);
> > void qemu_put_be32(QEMUFile *f, unsigned int v);
> > void qemu_put_be64(QEMUFile *f, uint64_t v);
> > size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);
> > +size_t qemu_get_buffer_to_pmem(QEMUFile *f, uint8_t *buf, size_t size);
> >
> > int qemu_get_byte(QEMUFile *f);
> >
> > diff --git a/include/qemu/pmem.h b/include/qemu/pmem.h
> > index 861d8ecc21..77ee1fc4eb 100644
> > --- a/include/qemu/pmem.h
> > +++ b/include/qemu/pmem.h
> > @@ -26,6 +26,12 @@ pmem_memcpy_persist(void *pmemdest, const void *src,
> > size_t len)
> > return memcpy(pmemdest, src, len);
> > }
> >
> > +static inline void *
> > +pmem_memcpy_nodrain(void *pmemdest, const void *src, size_t len)
> > +{
> > + return memcpy(pmemdest, src, len);
> > +}
> > +
> > static inline void *pmem_memset_nodrain(void *pmemdest, int c, size_t len)
> > {
> > return memset(pmemdest, c, len);
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index 2ab2bf362d..7e573010d9 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -26,6 +26,7 @@
> > #include "qemu-common.h"
> > #include "qemu/error-report.h"
> > #include "qemu/iov.h"
> > +#include "qemu/pmem.h"
> > #include "migration.h"
> > #include "qemu-file.h"
> > #include "trace.h"
> > @@ -471,15 +472,8 @@ size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf,
> > size_t size, size_t offset)
> > return size;
> > }
> >
> > -/*
> > - * Read 'size' bytes of data from the file into buf.
> > - * 'size' can be larger than the internal buffer.
> > - *
> > - * It will return size bytes unless there was an error, in which case it
> > will
> > - * return as many as it managed to read (assuming blocking fd's which
> > - * all current QEMUFile are)
> > - */
> > -size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
> > +static size_t
> > +qemu_get_buffer_common(QEMUFile *f, uint8_t *buf, size_t size, bool
> > is_pmem)
> > {
> > size_t pending = size;
> > size_t done = 0;
> > @@ -492,7 +486,11 @@ size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf,
> > size_t size)
> > if (res == 0) {
> > return done;
> > }
> > - memcpy(buf, src, res);
> > + if (!is_pmem) {
> > + memcpy(buf, src, res);
> > + } else {
> > + pmem_memcpy_nodrain(buf, src, res);
> > + }
>
> I see why you're doing it, but again I'm surprised it's ended up having
> to modify qemu-file.
Well, the current programming model of pmem requires users to take
special care on the data persistence, so QEMU (as a user of pmem) has
to do that as well.
>
> > qemu_file_skip(f, res);
> > buf += res;
> > pending -= res;
> > @@ -501,6 +499,29 @@ size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf,
> > size_t size)
> > return done;
> > }
> >
> > +/*
> > + * Read 'size' bytes of data from the file into buf.
> > + * 'size' can be larger than the internal buffer.
> > + *
> > + * It will return size bytes unless there was an error, in which case it
> > will
> > + * return as many as it managed to read (assuming blocking fd's which
> > + * all current QEMUFile are)
> > + */
> > +size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
> > +{
> > + return qemu_get_buffer_common(f, buf, size, false);
> > +}
> > +
> > +/*
> > + * Mostly the same as qemu_get_buffer(), except that
> > + * 1) it's for the case that 'buf' is in the persistent memory, and
> > + * 2) it takes necessary operations to ensure the data persistence in
> > 'buf'.
> > + */
> > +size_t qemu_get_buffer_to_pmem(QEMUFile *f, uint8_t *buf, size_t size)
> > +{
> > + return qemu_get_buffer_common(f, buf, size, true);
> > +}
> > +
> > /*
> > * Read 'size' bytes of data from the file.
> > * 'size' can be larger than the internal buffer.
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 5a0e503818..5a79bbff64 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -2950,7 +2950,11 @@ static int ram_load(QEMUFile *f, void *opaque, int
> > version_id)
> > break;
> >
> > case RAM_SAVE_FLAG_PAGE:
> > - qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> > + if (!is_pmem) {
> > + qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> > + } else {
> > + qemu_get_buffer_to_pmem(f, host, TARGET_PAGE_SIZE);
> > + }
>
> can you just expose qemu_get_buffer_common instead of making it static
> and just pass in the is_pmem flag.
Or perhaps passing a memcpyfunc function similarly to your comment on
patch 5.
Thanks,
Haozhong
>
> Dave
>
> > break;
> >
> > case RAM_SAVE_FLAG_COMPRESS_PAGE:
> > --
> > 2.14.1
> >
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v2 5/8] migration/ram: ensure write persistence on loading zero pages to PMEM, (continued)
Re: [Qemu-devel] [PATCH v2 5/8] migration/ram: ensure write persistence on loading zero pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
Re: [Qemu-devel] [PATCH v2 5/8] migration/ram: ensure write persistence on loading zero pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
[Qemu-devel] [PATCH v2 6/8] migration/ram: ensure write persistence on loading normal pages to PMEM, Haozhong Zhang, 2018/02/07
[Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Haozhong Zhang, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Haozhong Zhang, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Haozhong Zhang, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dan Williams, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dr. David Alan Gilbert, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dan Williams, 2018/02/07
- Re: [Qemu-devel] [PATCH v2 7/8] migration/ram: ensure write persistence on loading compressed pages to PMEM, Dr. David Alan Gilbert, 2018/02/07