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: Fri, 22 Nov 2019 00:37:00 -0500 (EST) [thread overview]
Message-ID: <838611538.35971353.1574401020319.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <CAPcyv4gsQXY5C5URF2vrTaD-0Q_CJ+ib3GVb1VFZAO+1Gdau2w@mail.gmail.com>
> > > > > >
> > > > > > 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?
> > > > >
> > > > > Why not just swap the parent child relationship in the PREFLUSH case?
> > > >
> > > > I we are already inside parent bio "make_request" function and we
> > > > create
> > > > child
> > > > bio. How we exactly will swap the parent/child relationship for
> > > > PREFLUSH
> > > > case?
> > > >
> > > > Child bio is queued after parent bio completes.
> > >
> > > Sorry, I didn't quite mean with bio_split, but issuing another request
> > > in front of the real bio. See md_flush_request() for inspiration.
> >
> > o.k. Thank you. Will try to post patch today to be considered for 5.4.
> >
>
> I think it is too late for v5.4-final, but we can get it in the
> -stable queue. Let's take the time to do it right and get some testing
> on it.
Sure.
Just sharing probable patch for early feedback, if I am doing it correctly?
I will test it thoroughly.
Thanks,
Pankaj
========
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 10351d5b49fa..c683e0e2515c 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -112,6 +112,12 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
bio_copy_dev(child, bio);
child->bi_opf = REQ_PREFLUSH;
child->bi_iter.bi_sector = -1;
+
+ if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
+ struct request_queue *q = bio->bi_disk->queue;
+ q->make_request_fn(q, child);
+ return 0;
+ }
bio_chain(child, bio);
submit_bio(child);
return 0;
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: Fri, 22 Nov 2019 00:37:00 -0500 (EST) [thread overview]
Message-ID: <838611538.35971353.1574401020319.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <CAPcyv4gsQXY5C5URF2vrTaD-0Q_CJ+ib3GVb1VFZAO+1Gdau2w@mail.gmail.com>
> > > > > >
> > > > > > 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?
> > > > >
> > > > > Why not just swap the parent child relationship in the PREFLUSH case?
> > > >
> > > > I we are already inside parent bio "make_request" function and we
> > > > create
> > > > child
> > > > bio. How we exactly will swap the parent/child relationship for
> > > > PREFLUSH
> > > > case?
> > > >
> > > > Child bio is queued after parent bio completes.
> > >
> > > Sorry, I didn't quite mean with bio_split, but issuing another request
> > > in front of the real bio. See md_flush_request() for inspiration.
> >
> > o.k. Thank you. Will try to post patch today to be considered for 5.4.
> >
>
> I think it is too late for v5.4-final, but we can get it in the
> -stable queue. Let's take the time to do it right and get some testing
> on it.
Sure.
Just sharing probable patch for early feedback, if I am doing it correctly?
I will test it thoroughly.
Thanks,
Pankaj
========
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 10351d5b49fa..c683e0e2515c 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -112,6 +112,12 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
bio_copy_dev(child, bio);
child->bi_opf = REQ_PREFLUSH;
child->bi_iter.bi_sector = -1;
+
+ if (unlikely(bio->bi_opf & REQ_PREFLUSH)) {
+ struct request_queue *q = bio->bi_disk->queue;
+ q->make_request_fn(q, child);
+ return 0;
+ }
bio_chain(child, bio);
submit_bio(child);
return 0;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
next prev parent reply other threads:[~2019-11-22 5:37 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 [this message]
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
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=838611538.35971353.1574401020319.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.