Re: [PATCH v4] virtio_pmem: add the missing REQ_OP_WRITE for flush bio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/4/23 14:03, 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@xxxxxxxxx>

Dave, will you queue this for 6.6.

Looks like it's already queued:
https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/commit/?h=libnvdimm-for-next&id=c1dbd8a849183b9c12d257ad3043ecec50db50b3




Thanks,
Pankaj

From: Hou Tao <houtao1@xxxxxxxxxx>

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@xxxxxxxxxxxxxxx # 6.3+
Fixes: b4a6bb3a67aa ("block: add a sanity check for non-write flush/fua bios")
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
Reviewed-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Tested-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
---
v4:
  * add stable Cc
  * collect Rvb and Tested-by tags

v3: https://lore.kernel.org/linux-block/20230625022633.2753877-1-houtao@xxxxxxxxxxxxxxx
  * adjust the overly long lines in both commit message and code

v2: https://lore.kernel.org/linux-block/20230621134340.878461-1-houtao@xxxxxxxxxxxxxxx
  * do a minimal fix first (Suggested by Christoph)

v1: https://lore.kernel.org/linux-block/ZJLpYMC8FgtZ0k2k@xxxxxxxxxxxxx/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





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux