Virtualization - Aug 2023 - [PATCH v4] virtio_pmem: add the missing REQ_OP_WRITE for flush bio

On Fri, Aug 04, 2023 at 09:03:20PM +0000, Verma, Vishal L
wrote:
> On Fri, 2023-08-04 at 20:39 +0200, Pankaj Gupta wrote:
> > Gentle ping!
> > 
> > Dan, Vishal for suggestion/review on this patch and request for
merging.
> > +Cc Michael for awareness, as virtio-pmem device is currently broken.
> 
> Looks good to me,
> 
> Reviewed-by: Vishal Verma <vishal.l.verma at intel.com>
> 
> Dave, will you queue this for 6.6.

Generally if you expect me to merge a patch I should be CC'd.

> > 
> > Thanks,
> > Pankaj
> > 
> > > From: Hou Tao <houtao1 at huawei.com>
> > > 
> > > When doing mkfs.xfs on a pmem device, the following warning was
> > > reported:
> > > 
> > > ?------------[ cut here ]------------
> > > ?WARNING: CPU: 2 PID: 384 at block/blk-core.c:751
submit_bio_noacct
> > > ?Modules linked in:
> > > ?CPU: 2 PID: 384 Comm: mkfs.xfs Not tainted 6.4.0-rc7+ #154
> > > ?Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
> > > ?RIP: 0010:submit_bio_noacct+0x340/0x520
> > > ?......
> > > ?Call Trace:
> > > ? <TASK>
> > > ? ? submit_bio_noacct+0xd5/0x520
> > > ? submit_bio+0x37/0x60
> > > ? async_pmem_flush+0x79/0xa0
> > > ? nvdimm_flush+0x17/0x40
> > > ? pmem_submit_bio+0x370/0x390
> > > ? __submit_bio+0xbc/0x190
> > > ? submit_bio_noacct_nocheck+0x14d/0x370
> > > ? submit_bio_noacct+0x1ef/0x520
> > > ? submit_bio+0x55/0x60
> > > ? submit_bio_wait+0x5a/0xc0
> > > ? blkdev_issue_flush+0x44/0x60
> > > 
> > > The root cause is that submit_bio_noacct() needs bio_op() is
either
> > > WRITE or ZONE_APPEND for flush bio and async_pmem_flush()
doesn't assign
> > > REQ_OP_WRITE when allocating flush bio, so submit_bio_noacct just
fail
> > > the flush bio.
> > > 
> > > Simply fix it by adding the missing REQ_OP_WRITE for flush bio.
And we
> > > could fix the flush order issue and do flush optimization later.
> > > 
> > > Cc: stable at vger.kernel.org?# 6.3+
> > > Fixes: b4a6bb3a67aa ("block: add a sanity check for
non-write flush/fua bios")
> > > Reviewed-by: Christoph Hellwig <hch at lst.de>
> > > Reviewed-by: Chaitanya Kulkarni <kch at nvidia.com>
> > > Reviewed-by: Pankaj Gupta <pankaj.gupta at amd.com>
> > > Tested-by: Pankaj Gupta <pankaj.gupta at amd.com>
> > > Signed-off-by: Hou Tao <houtao1 at huawei.com>
> > > ---
> > > v4:
> > > ?* add stable Cc
> > > ?* collect Rvb and Tested-by tags
> > > 
> > > v3:
https://lore.kernel.org/linux-block/20230625022633.2753877-1-houtao at
huaweicloud.com
> > > ?* adjust the overly long lines in both commit message and code
> > > 
> > > v2:
https://lore.kernel.org/linux-block/20230621134340.878461-1-houtao at
huaweicloud.com
> > > ?* do a minimal fix first (Suggested by Christoph)
> > > 
> > > v1: https://lore.kernel.org/linux-block/ZJLpYMC8FgtZ0k2k at
infradead.org/T/#t
> > > 
> > > ?drivers/nvdimm/nd_virtio.c | 3 ++-
> > > ?1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/nvdimm/nd_virtio.c
b/drivers/nvdimm/nd_virtio.c
> > > index c6a648fd8744..1f8c667c6f1e 100644
> > > --- a/drivers/nvdimm/nd_virtio.c
> > > +++ b/drivers/nvdimm/nd_virtio.c
> > > @@ -105,7 +105,8 @@ int async_pmem_flush(struct nd_region
*nd_region, struct bio *bio)
> > > ???????? * 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_PREFLUSH,
> > > +?????????????? struct bio *child = bio_alloc(bio->bi_bdev, 0,
> > > +???????????????????????????????????????????? REQ_OP_WRITE |
REQ_PREFLUSH,
> > > ????????????????????????????????????????????? GFP_ATOMIC);
> > > 
> > > ??????????????? if (!child)
> > > --
> > > 2.29.2
> > > 
>