All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pankaj Gupta <pagupta@redhat.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Jeff Moyer <jmoyer@redhat.com>,
	linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	Vishal L Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Vivek Goyal <vgoyal@redhat.com>,
	Keith Busch <keith.busch@intel.com>
Subject: Re: [PATCH] virtio pmem: fix async flush ordering
Date: Thu, 21 Nov 2019 03:01:56 -0500 (EST)	[thread overview]
Message-ID: <2089367516.35808121.1574323316486.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <CAPcyv4gCe8k1GdatAWn1991pm3QZq2WBFAGEFsZ2PXpyo2=wMw@mail.gmail.com>



> >
> > >  Remove logic to create child bio in the async flush function which
> > >  causes child bio to get executed after parent bio 'pmem_make_request'
> > >  completes. This resulted in wrong ordering of REQ_PREFLUSH with the
> > >  data write request.
> > >
> > >  Instead we are performing flush from the parent bio to maintain the
> > >  correct order. Also, returning from function 'pmem_make_request' if
> > >  REQ_PREFLUSH returns an error.
> > >
> > > Reported-by: Jeff Moyer <jmoyer@redhat.com>
> > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >
> > There's a slight change in behavior for the error path in the
> > virtio_pmem driver.  Previously, all errors from virtio_pmem_flush were
> > converted to -EIO.  Now, they are reported as-is.  I think this is
> > actually an improvement.
> >
> > I'll also note that the current behavior can result in data corruption,
> > so this should be tagged for stable.
> 
> I added that and was about to push this out, but what about the fact
> that now the guest will synchronously wait for flushing to occur. The
> goal of the child bio was to allow that to be an I/O wait with
> overlapping I/O, or at least not blocking the submission thread. Does
> the block layer synchronously wait for PREFLUSH requests? If not I
> think a synchronous wait is going to be a significant performance
> regression. Are there any numbers to accompany this change?

My bad, I missed this point completely.

Thanks,
Pankaj

> 
> 


WARNING: multiple messages have this Message-ID (diff)
From: Pankaj Gupta <pagupta@redhat.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-nvdimm <linux-nvdimm@lists.01.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>
Subject: Re: [PATCH] virtio pmem: fix async flush ordering
Date: Thu, 21 Nov 2019 03:01:56 -0500 (EST)	[thread overview]
Message-ID: <2089367516.35808121.1574323316486.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <CAPcyv4gCe8k1GdatAWn1991pm3QZq2WBFAGEFsZ2PXpyo2=wMw@mail.gmail.com>



> >
> > >  Remove logic to create child bio in the async flush function which
> > >  causes child bio to get executed after parent bio 'pmem_make_request'
> > >  completes. This resulted in wrong ordering of REQ_PREFLUSH with the
> > >  data write request.
> > >
> > >  Instead we are performing flush from the parent bio to maintain the
> > >  correct order. Also, returning from function 'pmem_make_request' if
> > >  REQ_PREFLUSH returns an error.
> > >
> > > Reported-by: Jeff Moyer <jmoyer@redhat.com>
> > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >
> > There's a slight change in behavior for the error path in the
> > virtio_pmem driver.  Previously, all errors from virtio_pmem_flush were
> > converted to -EIO.  Now, they are reported as-is.  I think this is
> > actually an improvement.
> >
> > I'll also note that the current behavior can result in data corruption,
> > so this should be tagged for stable.
> 
> I added that and was about to push this out, but what about the fact
> that now the guest will synchronously wait for flushing to occur. The
> goal of the child bio was to allow that to be an I/O wait with
> overlapping I/O, or at least not blocking the submission thread. Does
> the block layer synchronously wait for PREFLUSH requests? If not I
> think a synchronous wait is going to be a significant performance
> regression. Are there any numbers to accompany this change?

My bad, I missed this point completely.

Thanks,
Pankaj

> 
> 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

  parent reply	other threads:[~2019-11-21  8:02 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-20  9:28 [PATCH] virtio pmem: fix async flush ordering Pankaj Gupta
2019-11-20  9:28 ` Pankaj Gupta
2019-11-20 17:26 ` Jeff Moyer
2019-11-20 17:26   ` Jeff Moyer
2019-11-21  6:44   ` Pankaj Gupta
2019-11-21  6:44     ` Pankaj Gupta
2019-11-21  7:23   ` Dan Williams
2019-11-21  7:23     ` Dan Williams
2019-11-21  7:32     ` Dan Williams
2019-11-21  7:32       ` Dan Williams
2019-11-21  8:00       ` Pankaj Gupta
2019-11-21  8:00         ` Pankaj Gupta
2019-11-21 16:09         ` Dan Williams
2019-11-21 16:09           ` Dan Williams
2019-11-22  4:38           ` Pankaj Gupta
2019-11-22  4:38             ` Pankaj Gupta
2019-11-22  5:17             ` Dan Williams
2019-11-22  5:17               ` Dan Williams
2019-11-22  5:37               ` Pankaj Gupta
2019-11-22  5:37                 ` Pankaj Gupta
2019-11-22 22:52                 ` Dan Williams
2019-11-22 22:52                   ` Dan Williams
2019-11-21  8:01     ` Pankaj Gupta [this message]
2019-11-21  8:01       ` Pankaj Gupta
2019-11-22 16:08     ` Jeff Moyer
2019-11-22 16:08       ` Jeff Moyer
2019-11-22 16:13       ` Dan Williams
2019-11-22 16:13         ` Dan Williams
2019-11-22 16:25         ` Jeff Moyer
2019-11-22 16:25           ` Jeff Moyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2089367516.35808121.1574323316486.JavaMail.zimbra@redhat.com \
    --to=pagupta@redhat.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=jmoyer@redhat.com \
    --cc=keith.busch@intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=rjw@rjwysocki.net \
    --cc=vgoyal@redhat.com \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.