From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pankaj Gupta Subject: Re: [RFC v2 2/2] pmem: device flush over VIRTIO Date: Thu, 26 Apr 2018 13:13:44 -0400 (EDT) Message-ID: <1302242642.23016855.1524762824836.JavaMail.zimbra@redhat.com> References: <20180425112415.12327-1-pagupta@redhat.com> <20180425112415.12327-3-pagupta@redhat.com> <20180426131517.GB30991@stefanha-x1.localdomain> <58645254.23011245.1524760853269.JavaMail.zimbra@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Dan Williams Cc: Jan Kara , KVM list , David Hildenbrand , linux-nvdimm , ross zwisler , Qemu Developers , lcapitulino-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Linux MM , niteshnarayanlal-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org, "Michael S. Tsirkin" , Christoph Hellwig , Stefan Hajnoczi , Marcel Apfelbaum , Nitesh Narayan Lal , Rik van Riel , Stefan Hajnoczi , Paolo Bonzini , Kevin Wolf , xiaoguangrong eric , Linux Kernel Mailing List , Igor Mammedov List-Id: linux-nvdimm@lists.01.org > > > >> > >> On Wed, Apr 25, 2018 at 04:54:14PM +0530, Pankaj Gupta wrote: > >> > This patch adds functionality to perform > >> > flush from guest to hosy over VIRTIO > >> > when 'ND_REGION_VIRTIO'flag is set on > >> > nd_negion. Flag is set by 'virtio-pmem' > >> > driver. > >> > > >> > Signed-off-by: Pankaj Gupta > >> > --- > >> > drivers/nvdimm/region_devs.c | 7 +++++++ > >> > 1 file changed, 7 insertions(+) > >> > > >> > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c > >> > index a612be6..6c6454e 100644 > >> > --- a/drivers/nvdimm/region_devs.c > >> > +++ b/drivers/nvdimm/region_devs.c > >> > @@ -20,6 +20,7 @@ > >> > #include > >> > #include "nd-core.h" > >> > #include "nd.h" > >> > +#include > >> > > >> > /* > >> > * For readq() and writeq() on 32-bit builds, the hi-lo, lo-hi order is > >> > @@ -1074,6 +1075,12 @@ void nvdimm_flush(struct nd_region *nd_region) > >> > struct nd_region_data *ndrd = dev_get_drvdata(&nd_region->dev); > >> > int i, idx; > >> > > >> > + /* call PV device flush */ > >> > + if (test_bit(ND_REGION_VIRTIO, &nd_region->flags)) { > >> > + virtio_pmem_flush(&nd_region->dev); > >> > + return; > >> > + } > >> > >> How does libnvdimm know when flush has completed? > >> > >> Callers expect the flush to be finished when nvdimm_flush() returns but > >> the virtio driver has only queued the request, it hasn't waited for > >> completion! > > > > I tried to implement what nvdimm does right now. It just writes to > > flush hint address to make sure data persists. > > nvdimm_flush() is currently expected to be synchronous. Currently it > is sfence(); write to special address; sfence(). By the time the > second sfence returns the data is flushed. So you would need to make > this virtio flush interface synchronous as well, but that appears > problematic to stop the guest for unbounded amounts of time. Instead, > you need to rework nvdimm_flush() and the pmem driver to make these > flush requests asynchronous and add the plumbing for completion > callbacks via bio_endio(). o.k. > > > I just did not want to block guest write requests till host side > > fsync completes. > > You must complete the flush before bio_endio(), otherwise you're > violating the expectations of the guest filesystem/block-layer. sure! > > > > > be worse for operations on different guest files because all these > > operations would happen > > ultimately on same file at host. > > > > I think with current way, we can achieve an asynchronous queuing mechanism > > on cost of not > > 100% sure when fsync would complete but it is assured it will happen. Also, > > its entire block > > flush. > > No, again, that's broken. We need to add the plumbing for > communicating the fsync() completion relative the WRITE_{FLUSH,FUA} > bio in the guest. Sure. Thanks Dan & Stefan for the explanation and review. Best regards, Pankaj