* [RFC 1/2] pmem: make nvdimm_flush asynchronous
2020-04-20 13:19 [RFC 0/2] virtio-pmem: Asynchronous flush Pankaj Gupta
@ 2020-04-20 13:19 ` Pankaj Gupta
2020-04-20 13:19 ` [RFC 2/2] virtio_pmem: Async virtio-pmem flush Pankaj Gupta
2021-03-11 16:31 ` [RFC 0/2] virtio-pmem: Asynchronous flush David Hildenbrand
2 siblings, 0 replies; 7+ messages in thread
From: Pankaj Gupta @ 2020-04-20 13:19 UTC (permalink / raw)
To: linux-kernel, linux-nvdimm
Cc: dan.j.williams, vishal.l.verma, dave.jiang, david, mst, jmoyer,
pankaj.gupta, Pankaj Gupta
This patch converts nvdimm_flush to return when
asynchronous flush is in process.
Signed-off-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
---
drivers/nvdimm/pmem.c | 15 ++++++++-------
drivers/nvdimm/region_devs.c | 3 +--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 4ffc6f7ca131..747ffaee513b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -192,8 +192,10 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
struct pmem_device *pmem = q->queuedata;
struct nd_region *nd_region = to_region(pmem);
- if (bio->bi_opf & REQ_PREFLUSH)
- ret = nvdimm_flush(nd_region, bio);
+ if ((bio->bi_opf & REQ_PREFLUSH) &&
+ nvdimm_flush(nd_region, bio)) {
+ return BLK_QC_T_NONE;
+ }
do_acct = nd_iostat_start(bio, &start);
bio_for_each_segment(bvec, bio, iter) {
@@ -207,11 +209,10 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
if (do_acct)
nd_iostat_end(bio, start);
- if (bio->bi_opf & REQ_FUA)
- ret = nvdimm_flush(nd_region, bio);
-
- if (ret)
- bio->bi_status = errno_to_blk_status(ret);
+ if (bio->bi_opf & REQ_FUA) {
+ nvdimm_flush(nd_region, bio);
+ return BLK_QC_T_NONE;
+ }
bio_endio(bio);
return BLK_QC_T_NONE;
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index a19e535830d9..1caa13f1523f 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1091,8 +1091,7 @@ int nvdimm_flush(struct nd_region *nd_region, struct bio *bio)
if (!nd_region->flush)
rc = generic_nvdimm_flush(nd_region);
else {
- if (nd_region->flush(nd_region, bio))
- rc = -EIO;
+ rc = nd_region->flush(nd_region, bio);
}
return rc;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC 2/2] virtio_pmem: Async virtio-pmem flush
2020-04-20 13:19 [RFC 0/2] virtio-pmem: Asynchronous flush Pankaj Gupta
2020-04-20 13:19 ` [RFC 1/2] pmem: make nvdimm_flush asynchronous Pankaj Gupta
@ 2020-04-20 13:19 ` Pankaj Gupta
2021-03-11 16:31 ` [RFC 0/2] virtio-pmem: Asynchronous flush David Hildenbrand
2 siblings, 0 replies; 7+ messages in thread
From: Pankaj Gupta @ 2020-04-20 13:19 UTC (permalink / raw)
To: linux-kernel, linux-nvdimm
Cc: dan.j.williams, vishal.l.verma, dave.jiang, david, mst, jmoyer,
pankaj.gupta, Pankaj Gupta
This patch enables asynchronous flush for virtio pmem
using work queue. Also, it coalesce the flush requests
when a flush is already in process.
Signed-off-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
---
drivers/nvdimm/nd_virtio.c | 62 ++++++++++++++++++++++++++----------
drivers/nvdimm/virtio_pmem.c | 9 ++++++
drivers/nvdimm/virtio_pmem.h | 14 ++++++++
3 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
index 10351d5b49fa..ef53d0a0d134 100644
--- a/drivers/nvdimm/nd_virtio.c
+++ b/drivers/nvdimm/nd_virtio.c
@@ -97,29 +97,57 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
return err;
};
+static void submit_async_flush(struct work_struct *ws);
+
/* The asynchronous flush callback function */
int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
{
- /*
- * Create child bio for asynchronous flush and chain with
- * parent bio. Otherwise directly call nd_region flush.
- */
- if (bio && bio->bi_iter.bi_sector != -1) {
- struct bio *child = bio_alloc(GFP_ATOMIC, 0);
-
- if (!child)
- return -ENOMEM;
- bio_copy_dev(child, bio);
- child->bi_opf = REQ_PREFLUSH;
- child->bi_iter.bi_sector = -1;
- bio_chain(child, bio);
- submit_bio(child);
- return 0;
+ struct virtio_device *vdev = nd_region->provider_data;
+ struct virtio_pmem *vpmem = vdev->priv;
+ ktime_t start = ktime_get_boottime();
+
+ spin_lock_irq(&vpmem->lock);
+ wait_event_lock_irq(vpmem->sb_wait,
+ !vpmem->flush_bio ||
+ ktime_after(vpmem->last_flush, start),
+ vpmem->lock);
+
+ if (!ktime_after(vpmem->last_flush, start)) {
+ WARN_ON(vpmem->flush_bio);
+ vpmem->flush_bio = bio;
+ bio = NULL;
}
- if (virtio_pmem_flush(nd_region))
- return -EIO;
+ spin_unlock_irq(&vpmem->lock);
+ if (!bio) {
+ INIT_WORK(&vpmem->flush_work, submit_async_flush);
+ queue_work(vpmem->pmem_wq, &vpmem->flush_work);
+ return 1;
+ }
+
+ bio->bi_opf &= ~REQ_PREFLUSH;
return 0;
};
EXPORT_SYMBOL_GPL(async_pmem_flush);
+
+static void submit_async_flush(struct work_struct *ws)
+{
+ struct virtio_pmem *vpmem = container_of(ws, struct virtio_pmem, flush_work);
+ struct bio *bio = vpmem->flush_bio;
+
+ vpmem->start_flush = ktime_get_boottime();
+ bio->bi_status = errno_to_blk_status(virtio_pmem_flush(vpmem->nd_region));
+ vpmem->last_flush = vpmem->start_flush;
+ vpmem->flush_bio = NULL;
+ wake_up(&vpmem->sb_wait);
+
+ /* Submit parent bio only for PREFLUSH */
+ if (bio && (bio->bi_opf & REQ_PREFLUSH)) {
+ bio->bi_opf &= ~REQ_PREFLUSH;
+ submit_bio(bio);
+ } else if (bio && (bio->bi_opf & REQ_FUA)) {
+ bio->bi_opf &= ~REQ_FUA;
+ bio_endio(bio);
+ }
+}
MODULE_LICENSE("GPL");
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 5e3d07b47e0c..4ab135d820fd 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -24,6 +24,7 @@ static int init_vq(struct virtio_pmem *vpmem)
return PTR_ERR(vpmem->req_vq);
spin_lock_init(&vpmem->pmem_lock);
+ spin_lock_init(&vpmem->lock);
INIT_LIST_HEAD(&vpmem->req_list);
return 0;
@@ -58,6 +59,12 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
goto out_err;
}
+ vpmem->pmem_wq = alloc_workqueue("vpmem_wq", WQ_MEM_RECLAIM, 0);
+ if (!vpmem->pmem_wq) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+ init_waitqueue_head(&vpmem->sb_wait);
virtio_cread(vpmem->vdev, struct virtio_pmem_config,
start, &vpmem->start);
virtio_cread(vpmem->vdev, struct virtio_pmem_config,
@@ -90,10 +97,12 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
goto out_nd;
}
nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
+ vpmem->nd_region = nd_region;
return 0;
out_nd:
nvdimm_bus_unregister(vpmem->nvdimm_bus);
out_vq:
+ destroy_workqueue(vpmem->pmem_wq);
vdev->config->del_vqs(vdev);
out_err:
return err;
diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
index 0dddefe594c4..9d3615a324bf 100644
--- a/drivers/nvdimm/virtio_pmem.h
+++ b/drivers/nvdimm/virtio_pmem.h
@@ -35,9 +35,23 @@ struct virtio_pmem {
/* Virtio pmem request queue */
struct virtqueue *req_vq;
+ struct bio *flush_bio;
+ /* last_flush is when the last completed flush was started */
+ ktime_t last_flush, start_flush;
+
+ /* work queue for deferred flush */
+ struct work_struct flush_work;
+ struct workqueue_struct *pmem_wq;
+
+ /* Synchronize flush wait queue data */
+ spinlock_t lock;
+ /* for waiting for previous flush to complete */
+ wait_queue_head_t sb_wait;
+
/* nvdimm bus registers virtio pmem device */
struct nvdimm_bus *nvdimm_bus;
struct nvdimm_bus_descriptor nd_desc;
+ struct nd_region *nd_region;
/* List to store deferred work if virtqueue is full */
struct list_head req_list;
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC 0/2] virtio-pmem: Asynchronous flush
2020-04-20 13:19 [RFC 0/2] virtio-pmem: Asynchronous flush Pankaj Gupta
2020-04-20 13:19 ` [RFC 1/2] pmem: make nvdimm_flush asynchronous Pankaj Gupta
2020-04-20 13:19 ` [RFC 2/2] virtio_pmem: Async virtio-pmem flush Pankaj Gupta
@ 2021-03-11 16:31 ` David Hildenbrand
2021-03-12 4:21 ` Pankaj Gupta
2 siblings, 1 reply; 7+ messages in thread
From: David Hildenbrand @ 2021-03-11 16:31 UTC (permalink / raw)
To: Pankaj Gupta, linux-kernel, linux-nvdimm
Cc: dan.j.williams, vishal.l.verma, dave.jiang, mst, jmoyer, pankaj.gupta
On 20.04.20 15:19, Pankaj Gupta wrote:
> Jeff reported preflush order issue with the existing implementation
> of virtio pmem preflush. Dan suggested[1] to implement asynchronous flush
> for virtio pmem using work queue as done in md/RAID. This patch series
> intends to solve the preflush ordering issue and also makes the flush
> asynchronous from the submitting thread POV.
>
> Submitting this patch series for feeback and is in WIP. I have
> done basic testing and currently doing more testing.
>
> Pankaj Gupta (2):
> pmem: make nvdimm_flush asynchronous
> virtio_pmem: Async virtio-pmem flush
>
> drivers/nvdimm/nd_virtio.c | 66 ++++++++++++++++++++++++++----------
> drivers/nvdimm/pmem.c | 15 ++++----
> drivers/nvdimm/region_devs.c | 3 +-
> drivers/nvdimm/virtio_pmem.c | 9 +++++
> drivers/nvdimm/virtio_pmem.h | 12 +++++++
> 5 files changed, 78 insertions(+), 27 deletions(-)
>
> [1] https://marc.info/?l=linux-kernel&m=157446316409937&w=2
>
Just wondering, was there any follow up of this or are we still waiting
for feedback? :)
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC 0/2] virtio-pmem: Asynchronous flush
2021-03-11 16:31 ` [RFC 0/2] virtio-pmem: Asynchronous flush David Hildenbrand
@ 2021-03-12 4:21 ` Pankaj Gupta
2021-03-12 6:02 ` Dan Williams
0 siblings, 1 reply; 7+ messages in thread
From: Pankaj Gupta @ 2021-03-12 4:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: Pankaj Gupta, linux-kernel, linux-nvdimm, dan.j.williams,
vishal.l.verma, dave.jiang, mst, jmoyer
Hi David,
> > Jeff reported preflush order issue with the existing implementation
> > of virtio pmem preflush. Dan suggested[1] to implement asynchronous flush
> > for virtio pmem using work queue as done in md/RAID. This patch series
> > intends to solve the preflush ordering issue and also makes the flush
> > asynchronous from the submitting thread POV.
> >
> > Submitting this patch series for feeback and is in WIP. I have
> > done basic testing and currently doing more testing.
> >
> > Pankaj Gupta (2):
> > pmem: make nvdimm_flush asynchronous
> > virtio_pmem: Async virtio-pmem flush
> >
> > drivers/nvdimm/nd_virtio.c | 66 ++++++++++++++++++++++++++----------
> > drivers/nvdimm/pmem.c | 15 ++++----
> > drivers/nvdimm/region_devs.c | 3 +-
> > drivers/nvdimm/virtio_pmem.c | 9 +++++
> > drivers/nvdimm/virtio_pmem.h | 12 +++++++
> > 5 files changed, 78 insertions(+), 27 deletions(-)
> >
> > [1] https://marc.info/?l=linux-kernel&m=157446316409937&w=2
> >
>
> Just wondering, was there any follow up of this or are we still waiting
> for feedback? :)
Thank you for bringing this up.
My apologies I could not followup on this. I have another version in my local
tree but could not post it as I was not sure if I solved the problem
correctly. I will
clean it up and post for feedback as soon as I can.
P.S: Due to serious personal/family health issues I am not able to
devote much time
on this with other professional commitments. I feel bad that I have
this unfinished task.
Just in last one year things have not been stable for me & my family
and still not getting :(
Best regards,
Pankaj
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC 0/2] virtio-pmem: Asynchronous flush
2021-03-12 4:21 ` Pankaj Gupta
@ 2021-03-12 6:02 ` Dan Williams
2021-03-12 8:53 ` David Hildenbrand
0 siblings, 1 reply; 7+ messages in thread
From: Dan Williams @ 2021-03-12 6:02 UTC (permalink / raw)
To: Pankaj Gupta
Cc: David Hildenbrand, Pankaj Gupta, Linux Kernel Mailing List,
linux-nvdimm, Vishal L Verma, Dave Jiang, Michael S. Tsirkin,
jmoyer
On Thu, Mar 11, 2021 at 8:21 PM Pankaj Gupta
<pankaj.gupta@cloud.ionos.com> wrote:
>
> Hi David,
>
> > > Jeff reported preflush order issue with the existing implementation
> > > of virtio pmem preflush. Dan suggested[1] to implement asynchronous flush
> > > for virtio pmem using work queue as done in md/RAID. This patch series
> > > intends to solve the preflush ordering issue and also makes the flush
> > > asynchronous from the submitting thread POV.
> > >
> > > Submitting this patch series for feeback and is in WIP. I have
> > > done basic testing and currently doing more testing.
> > >
> > > Pankaj Gupta (2):
> > > pmem: make nvdimm_flush asynchronous
> > > virtio_pmem: Async virtio-pmem flush
> > >
> > > drivers/nvdimm/nd_virtio.c | 66 ++++++++++++++++++++++++++----------
> > > drivers/nvdimm/pmem.c | 15 ++++----
> > > drivers/nvdimm/region_devs.c | 3 +-
> > > drivers/nvdimm/virtio_pmem.c | 9 +++++
> > > drivers/nvdimm/virtio_pmem.h | 12 +++++++
> > > 5 files changed, 78 insertions(+), 27 deletions(-)
> > >
> > > [1] https://marc.info/?l=linux-kernel&m=157446316409937&w=2
> > >
> >
> > Just wondering, was there any follow up of this or are we still waiting
> > for feedback? :)
>
> Thank you for bringing this up.
>
> My apologies I could not followup on this. I have another version in my local
> tree but could not post it as I was not sure if I solved the problem
> correctly. I will
> clean it up and post for feedback as soon as I can.
>
> P.S: Due to serious personal/family health issues I am not able to
> devote much time
> on this with other professional commitments. I feel bad that I have
> this unfinished task.
> Just in last one year things have not been stable for me & my family
> and still not getting :(
No worries Pankaj. Take care of yourself and your family. The
community can handle this for you. I'm open to coaching somebody
through what's involved to get this fix landed.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC 0/2] virtio-pmem: Asynchronous flush
2021-03-12 6:02 ` Dan Williams
@ 2021-03-12 8:53 ` David Hildenbrand
0 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand @ 2021-03-12 8:53 UTC (permalink / raw)
To: Dan Williams, Pankaj Gupta
Cc: Pankaj Gupta, Linux Kernel Mailing List, linux-nvdimm,
Vishal L Verma, Dave Jiang, Michael S. Tsirkin, jmoyer
On 12.03.21 07:02, Dan Williams wrote:
> On Thu, Mar 11, 2021 at 8:21 PM Pankaj Gupta
> <pankaj.gupta@cloud.ionos.com> wrote:
>>
>> Hi David,
>>
>>>> Jeff reported preflush order issue with the existing implementation
>>>> of virtio pmem preflush. Dan suggested[1] to implement asynchronous flush
>>>> for virtio pmem using work queue as done in md/RAID. This patch series
>>>> intends to solve the preflush ordering issue and also makes the flush
>>>> asynchronous from the submitting thread POV.
>>>>
>>>> Submitting this patch series for feeback and is in WIP. I have
>>>> done basic testing and currently doing more testing.
>>>>
>>>> Pankaj Gupta (2):
>>>> pmem: make nvdimm_flush asynchronous
>>>> virtio_pmem: Async virtio-pmem flush
>>>>
>>>> drivers/nvdimm/nd_virtio.c | 66 ++++++++++++++++++++++++++----------
>>>> drivers/nvdimm/pmem.c | 15 ++++----
>>>> drivers/nvdimm/region_devs.c | 3 +-
>>>> drivers/nvdimm/virtio_pmem.c | 9 +++++
>>>> drivers/nvdimm/virtio_pmem.h | 12 +++++++
>>>> 5 files changed, 78 insertions(+), 27 deletions(-)
>>>>
>>>> [1] https://marc.info/?l=linux-kernel&m=157446316409937&w=2
>>>>
>>>
>>> Just wondering, was there any follow up of this or are we still waiting
>>> for feedback? :)
>>
>> Thank you for bringing this up.
>>
>> My apologies I could not followup on this. I have another version in my local
>> tree but could not post it as I was not sure if I solved the problem
>> correctly. I will
>> clean it up and post for feedback as soon as I can.
>>
>> P.S: Due to serious personal/family health issues I am not able to
>> devote much time
>> on this with other professional commitments. I feel bad that I have
>> this unfinished task.
>> Just in last one year things have not been stable for me & my family
>> and still not getting :(
>
> No worries Pankaj. Take care of yourself and your family. The
> community can handle this for you. I'm open to coaching somebody
> through what's involved to get this fix landed.
Absolutely, no need to worry for now - take care of yourself and your
loved ones! I was merely stumbling over this series while cleaning up my
inbox, wondering if this is still stuck waiting for review/feedback. No
need to rush anything or be stressed.
In case I have time to look into this in the future, I'd coordinate in
this thread (especially, asking for feedback again so I know where this
series stands)!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 7+ messages in thread