> pmem_submit_bio() passes the parent bio to nvdimm_flush() for > REQ_FUA. For virtio-pmem this makes async_pmem_flush() allocate > and submit a child PREFLUSH bio chained to the parent. > > That child allocation is in the block submit path. Making it > blocking with GFP_NOIO can consume the same global bio mempool that > submit_bio() uses, while making it GFP_ATOMIC can fail under > pressure. A forced failure of the child allocation produced: > > virtio_pmem: forcing child bio allocation failure for test > Buffer I/O error on dev pmem0, logical block 0, lost sync page write > EXT4-fs (pmem0): I/O error while writing superblock > EXT4-fs (pmem0): mount failed > > Avoid the child bio completely. Flush FUA synchronously, like > REQ_PREFLUSH, then complete the parent after the flush. Since no > child bio can be created, async_pmem_flush() now only issues the > virtio flush and preserves negative errno values.
Child flush is asynchronous (performs async flush to host side and returns). Till child bio completes guest userspace waits in pending IO state. It seems the current change will affect the behavior? Prior RFC [1] attempted to coalesce the async FLUSH request between guest &host. If there is interest, that approach could be revisited or integrated here? [1] https://lore.kernel.org/all/[email protected]/#t Thanks, Pankaj > > Signed-off-by: Li Chen <[email protected]> > --- > Changes in v6: > - Replace the child bio allocation fix with synchronous FUA flushing. > > drivers/nvdimm/nd_virtio.c | 22 ++++------------------ > drivers/nvdimm/pmem.c | 2 +- > 2 files changed, 5 insertions(+), 19 deletions(-) > > diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c > index 4176046627beb..4b2e9c47af0f5 100644 > --- a/drivers/nvdimm/nd_virtio.c > +++ b/drivers/nvdimm/nd_virtio.c > @@ -110,27 +110,13 @@ static int virtio_pmem_flush(struct nd_region > *nd_region) > /* 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(bio->bi_bdev, 0, > - REQ_OP_WRITE | REQ_PREFLUSH, > - GFP_ATOMIC); > + int err; > > - if (!child) > - return -ENOMEM; > - bio_clone_blkg_association(child, bio); > - child->bi_iter.bi_sector = -1; > - bio_chain(child, bio); > - submit_bio(child); > - return 0; > - } > - if (virtio_pmem_flush(nd_region)) > + err = virtio_pmem_flush(nd_region); > + if (err > 0) > return -EIO; > > - return 0; > + return err; > }; > EXPORT_SYMBOL_GPL(async_pmem_flush); > MODULE_DESCRIPTION("Virtio Persistent Memory Driver"); > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c > index 82ee1ddb3a445..058d2739c95a1 100644 > --- a/drivers/nvdimm/pmem.c > +++ b/drivers/nvdimm/pmem.c > @@ -242,7 +242,7 @@ static void pmem_submit_bio(struct bio *bio) > } > > if ((bio->bi_opf & REQ_FUA) && !bio->bi_status) > - ret = nvdimm_flush(nd_region, bio); > + ret = nvdimm_flush(nd_region, NULL); > > if (ret) > bio->bi_status = errno_to_blk_status(ret); > -- > 2.52.0
