| Message ID | 20170331084147.32716-3-haozhong.zhang@intel.com |
|---|---|
| State | New |
| Headers | show |
On 03/31/2017 04:41 PM, Haozhong Zhang wrote: > fsync() is used to persist modifications to the back store. If the > host NVDIMM is used as the back store, fsync() on Linux will trigger > the write to the host flush hint address. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ > include/hw/mem/nvdimm.h | 13 +++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index db896b0..484ab8b 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) > return &nvdimm->nvdimm_mr; > } > > +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) > +{ > + if (nvdimm->flush_hint_enabled) { > + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); Hmm, IIRC host-mem-file does not initalize backend_fd at all.
On 04/06/17 19:52 +0800, Xiao Guangrong wrote: > > > On 03/31/2017 04:41 PM, Haozhong Zhang wrote: > > fsync() is used to persist modifications to the back store. If the > > host NVDIMM is used as the back store, fsync() on Linux will trigger > > the write to the host flush hint address. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ > > include/hw/mem/nvdimm.h | 13 +++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > > index db896b0..484ab8b 100644 > > --- a/hw/mem/nvdimm.c > > +++ b/hw/mem/nvdimm.c > > @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) > > return &nvdimm->nvdimm_mr; > > } > > > > +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) > > +{ > > + if (nvdimm->flush_hint_enabled) { > > + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); > > Hmm, IIRC host-mem-file does not initalize backend_fd at all. Oops, forgot to add this part. Thanks for remind. Haozhong
On 04/11/17 16:22 +0800, Haozhong Zhang wrote: > On 04/06/17 19:52 +0800, Xiao Guangrong wrote: > > > > > > On 03/31/2017 04:41 PM, Haozhong Zhang wrote: > > > fsync() is used to persist modifications to the back store. If the > > > host NVDIMM is used as the back store, fsync() on Linux will trigger > > > the write to the host flush hint address. > > > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > > --- > > > hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ > > > include/hw/mem/nvdimm.h | 13 +++++++++++++ > > > 2 files changed, 35 insertions(+) > > > > > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > > > index db896b0..484ab8b 100644 > > > --- a/hw/mem/nvdimm.c > > > +++ b/hw/mem/nvdimm.c > > > @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) > > > return &nvdimm->nvdimm_mr; > > > } > > > > > > +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) > > > +{ > > > + if (nvdimm->flush_hint_enabled) { > > > + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); > > > > Hmm, IIRC host-mem-file does not initalize backend_fd at all. > > Oops, forgot to add this part. Thanks for remind. Sorry, I replied too quick. For hostmem-file, hostmem_mr->ram_block->fd is set in file_ram_alloc(), which can be got by memory_region_get_fd() here. Haozhong
On 04/11/2017 04:29 PM, Haozhong Zhang wrote: > On 04/11/17 16:22 +0800, Haozhong Zhang wrote: >> On 04/06/17 19:52 +0800, Xiao Guangrong wrote: >>> >>> >>> On 03/31/2017 04:41 PM, Haozhong Zhang wrote: >>>> fsync() is used to persist modifications to the back store. If the >>>> host NVDIMM is used as the back store, fsync() on Linux will trigger >>>> the write to the host flush hint address. >>>> >>>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >>>> --- >>>> hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ >>>> include/hw/mem/nvdimm.h | 13 +++++++++++++ >>>> 2 files changed, 35 insertions(+) >>>> >>>> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c >>>> index db896b0..484ab8b 100644 >>>> --- a/hw/mem/nvdimm.c >>>> +++ b/hw/mem/nvdimm.c >>>> @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) >>>> return &nvdimm->nvdimm_mr; >>>> } >>>> >>>> +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) >>>> +{ >>>> + if (nvdimm->flush_hint_enabled) { >>>> + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); >>> >>> Hmm, IIRC host-mem-file does not initalize backend_fd at all. >> >> Oops, forgot to add this part. Thanks for remind. > > Sorry, I replied too quick. > > For hostmem-file, hostmem_mr->ram_block->fd is set in file_ram_alloc(), > which can be got by memory_region_get_fd() here. Okay, my wrong memory and did not check the code during patch review. :)
On Fri, 31 Mar 2017 16:41:45 +0800 Haozhong Zhang <haozhong.zhang@intel.com> wrote: > fsync() is used to persist modifications to the back store. If the s/back/backing/ > host NVDIMM is used as the back store, fsync() on Linux will trigger > the write to the host flush hint address. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ > include/hw/mem/nvdimm.h | 13 +++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index db896b0..484ab8b 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) > return &nvdimm->nvdimm_mr; > } > > +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) > +{ > + if (nvdimm->flush_hint_enabled) { > + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); > + } else { > + nvdimm->backend_fd = -1; > + } > +} > + > +void nvdimm_flush(NVDIMMDevice *nvdimm) > +{ > + if (nvdimm->backend_fd != -1) { if backend isn't file and user asked for flush_hint, he/she silently won't get it. maybe we should fail nvdimm_realize() if backend is not file. > + /* > + * If the backend store is a physical NVDIMM device, fsync() > + * will trigger the flush via the flush hint on the host device. > + */ > + fsync(nvdimm->backend_fd); > + } > +} > + > static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > { > MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem, errp); > @@ -105,6 +125,8 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm), > "nvdimm-memory", mr, 0, pmem_size); > nvdimm->nvdimm_mr.align = align; > + > + nvdimm_flush_init(nvdimm, mr); > } > > /* > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h > index 03e1ff9..eb71f41 100644 > --- a/include/hw/mem/nvdimm.h > +++ b/include/hw/mem/nvdimm.h > @@ -71,6 +71,18 @@ struct NVDIMMDevice { > * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported. > */ > MemoryRegion nvdimm_mr; > + > + /* > + * If true, a flush hint address structure will be built for this > + * NVDIMM device. > + */ > + bool flush_hint_enabled; > + /* > + * File descriptor of the backend store, which is used in nvdimm > + * flush. It's cached in NVDIMMDevice rather than being fetched > + * at each request in order to accelerate the flush a little bit. > + */ > + int backend_fd; > }; > typedef struct NVDIMMDevice NVDIMMDevice; > > @@ -132,4 +144,5 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, > uint32_t ram_slots); > void nvdimm_plug(AcpiNVDIMMState *state); > void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev); > +void nvdimm_flush(NVDIMMDevice *nvdimm); > #endif
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index db896b0..484ab8b 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -78,6 +78,26 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm) return &nvdimm->nvdimm_mr; } +static void nvdimm_flush_init(NVDIMMDevice *nvdimm, MemoryRegion *hostmem_mr) +{ + if (nvdimm->flush_hint_enabled) { + nvdimm->backend_fd = memory_region_get_fd(hostmem_mr); + } else { + nvdimm->backend_fd = -1; + } +} + +void nvdimm_flush(NVDIMMDevice *nvdimm) +{ + if (nvdimm->backend_fd != -1) { + /* + * If the backend store is a physical NVDIMM device, fsync() + * will trigger the flush via the flush hint on the host device. + */ + fsync(nvdimm->backend_fd); + } +} + static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) { MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem, errp); @@ -105,6 +125,8 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm), "nvdimm-memory", mr, 0, pmem_size); nvdimm->nvdimm_mr.align = align; + + nvdimm_flush_init(nvdimm, mr); } /* diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h index 03e1ff9..eb71f41 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -71,6 +71,18 @@ struct NVDIMMDevice { * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported. */ MemoryRegion nvdimm_mr; + + /* + * If true, a flush hint address structure will be built for this + * NVDIMM device. + */ + bool flush_hint_enabled; + /* + * File descriptor of the backend store, which is used in nvdimm + * flush. It's cached in NVDIMMDevice rather than being fetched + * at each request in order to accelerate the flush a little bit. + */ + int backend_fd; }; typedef struct NVDIMMDevice NVDIMMDevice; @@ -132,4 +144,5 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data, uint32_t ram_slots); void nvdimm_plug(AcpiNVDIMMState *state); void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev); +void nvdimm_flush(NVDIMMDevice *nvdimm); #endif
fsync() is used to persist modifications to the back store. If the host NVDIMM is used as the back store, fsync() on Linux will trigger the write to the host flush hint address. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- hw/mem/nvdimm.c | 22 ++++++++++++++++++++++ include/hw/mem/nvdimm.h | 13 +++++++++++++ 2 files changed, 35 insertions(+)