diff mbox series

[v2] md/bitmap: Avoid protection error writing bitmap page with block integrity

Message ID 20230720092957epcms2p6cbd0ecdf7f5bf0db3c04eefaf0a6644d@epcms2p6
State New
Headers
Series [v2] md/bitmap: Avoid protection error writing bitmap page with block integrity |

Commit Message

Jinyoung Choi July 20, 2023, 9:29 a.m. UTC 
  Changing the bitmap page is also possible on the page where the DMA is
being performed or scheduled in the MD.

When configuring raid1 (mirror) with devices that support block integrity,
the same bitmap page is sent to the device twice during the resync process,
causing the following problems.
(When requeue is executed, integrity is not updated)

             [Func 1]                          [Func 2]

1     A(page) + a(integrity)
2        (sq doorbell)
3                                          A(page) -> A-1(page)
4  A-1(page-updated) + a(integrity)     A-1(page) + a-1(integrity)
5                                            (sq doorbell)
6           (DMA)                                (DMA)

       I/O Fail and retry N                   I/O Success
       To be Faulty Device

My Test Env: Two FIPS PM1743
- nvme format /dev/nvme2n1 --force -n 1 -i 1 -p 0 -m 0 -l 1 -r
- nvme format /dev/nvme3n1 --force -n 1 -i 1 -p 0 -m 0 -l 1 -r
- mdadm -C /dev/md0 -l 1 -n 2 /dev/nvme2n1 /dev/nvme3n1

The following is the log when a problem occurs. The problematic device
is in the faulty device state.

Log:
[  135.037253] md/raid1:md0: active with 2 out of 2 mirrors
[  135.038228] md0: detected capacity change from 0 to 7501212288
[  135.038270] md: resync of RAID array md0
[  151.252172] nvme2n1: I/O Cmd(0x1) @ LBA 16, 8 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[  151.252180] protection error, dev nvme2n1, sector 16 op 0x1:(WRITE) flags 0x10800 phys_seg 1 prio class 2
[  151.252185] md: super_written gets error=-84
[  151.252187] md/raid1:md0: Disk failure on nvme2n1, disabling device.
               md/raid1:md0: Operation continuing on 1 devices.
[  151.267450] nvme3n1: I/O Cmd(0x1) @ LBA 16, 8 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[  151.267457] protection error, dev nvme3n1, sector 16 op 0x1:(WRITE) flags 0x10800 phys_seg 1 prio class 2
[  151.267460] md: super_written gets error=-84
[  151.268458] md: md0: resync interrupted.
[  151.320765] md: resync of RAID array md0
[  151.321205] md: md0: resync done.

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 drivers/md/md-bitmap.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Christoph Hellwig July 31, 2023, 7:26 a.m. UTC | #1 
On Thu, Jul 20, 2023 at 06:29:57PM +0900, Jinyoung Choi wrote:
> Changing the bitmap page is also possible on the page where the DMA is
> being performed or scheduled in the MD.

Are you guys actually using the whacky bitmap feature?  If so, this
really is the least of our problems and you need to invest some real
work into it to get it off ->bmap and into using ->read_iter and
->write_iter.  Right now Song has a series from me queued up to
deprecated it.

I also don't think a simple wait for existing writers prevents you
from other modifications, but maybe I'm issing something.
Jinyoung Choi July 31, 2023, 7:49 a.m. UTC | #2 
> On Thu, Jul 20, 2023 at 06:29:57PM +0900, Jinyoung Choi wrote:
> > Changing the bitmap page is also possible on the page where the DMA is
> > being performed or scheduled in the MD.
> 
> Are you guys actually using the whacky bitmap feature?  If so, this
> really is the least of our problems and you need to invest some real
> work into it to get it off ->bmap and into using ->read_iter and
> ->write_iter.  Right now Song has a series from me queued up to
> deprecated it.
> 
> I also don't think a simple wait for existing writers prevents you
> from other modifications, but maybe I'm issing something.

Hi, Christoph.

I think the way to write bitmap page like you should be changed.
This patch is a simple workaround in the current system.

I will check the patches you provided for bitmap processing.

Thank you for your review.

Best Regards,
Jinyoung.
diff mbox series

Patch

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 1ff712889a3b..dfb7418ba48a 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -467,6 +467,13 @@  void md_bitmap_update_sb(struct bitmap *bitmap)
 		return;
 	if (!bitmap->storage.sb_page) /* no superblock */
 		return;
+
+	/*
+	 * Before modifying the bitmap page and re-issue it, wait for
+	 * the requests previously sent to the device to be completed.
+	 */
+	md_bitmap_wait_writes(bitmap);
+
 	sb = kmap_atomic(bitmap->storage.sb_page);
 	sb->events = cpu_to_le64(bitmap->mddev->events);
 	if (bitmap->mddev->events < bitmap->events_cleared)