block/048: add a lost mmap write test for block devices - #258
Conversation
When a folio holds more than one block, block device writeback tracks dirty state per block, so dirtying a folio through a shared mapping has to update that state and not just PG_dirty. When it does not, writeback finds no dirty range, submits no I/O and clears PG_dirty, resulting in data loss. This is a regression test for that issue, fixed in the kernel patch "block: use iomap_dirty_folio for block devices" [1]. Write a known pattern to a memory-backed null_blk device with O_DIRECT, store a different one through mmap(), msync() and fsync(), then read the range back with O_DIRECT and compare. Configure the device with a 512 byte block size so that a folio holds several blocks. With a single block per folio there is no per-block state to get out of sync and the write is never lost. Block devices only take the affected writeback path with CONFIG_BUFFER_HEAD=n. [1]: https://lore.kernel.org/linux-block/20260802-blkdev-fixes-v1-1-a82fc549fd74@columbia.edu/ Signed-off-by: Tal Zussman <tz2294@columbia.edu>
kawasaki
left a comment
There was a problem hiding this comment.
@tzussman Thanks for the PR. Overall, the code changes look good to me. I ran the test case and confirmed it recreates the failure, and the kernel fix patch avoids the failure. Good from testing point of view.
I left two comments. Please see if they make sense for you. Thanks!
| . tests/block/rc | ||
| . common/null_blk | ||
|
|
||
| DESCRIPTION="check that mmap writes to a block device with a small block size are not lost" |
There was a problem hiding this comment.
This description is printed in the console, so I suggest to make it shorter. How about "mmap writes smaller than folio size"?
| requires() { | ||
| _have_null_blk | ||
| _have_src_program mmap-loss | ||
| } |
There was a problem hiding this comment.
I needed to disable the kernel config BUFFER_HEAD to make this test case fail. Is it reasonable to skip this test case when the config BUFFER_HEAD is enabled?
If so, I suggest to add one more preparation commit with the change below:
diff --git a/common/rc b/common/rc
index 6500533..8f4bfeb 100644
--- a/common/rc
+++ b/common/rc
@@ -255,6 +255,18 @@ _have_kernel_option() {
return 0
}
+_does_not_have_kernel_option() {
+ local opt=$1
+
+ _have_kernel_config_file || return
+ if _check_kernel_option "$opt"; then
+ SKIP_REASONS+=("kernel option $opt is enabled")
+ return 1
+ fi
+
+ return 0
+}
+
_have_kernel_options() {
local ret=0
And we can call "_does_not_have_kernel_option BUFFER_HEAD" in requires().
When a folio holds more than one block, block device writeback tracks dirty state per block, so dirtying a folio through a shared mapping has to update that state and not just PG_dirty. When it does not, writeback finds no dirty range, submits no I/O and clears PG_dirty, resulting in data loss.
This is a regression test for that issue, fixed in the kernel patch "block: use iomap_dirty_folio for block devices" (1).
Write a known pattern to a memory-backed null_blk device with O_DIRECT, store a different one through mmap(), msync() and fsync(), then read the range back with O_DIRECT and compare. Configure the device with a 512 byte block size so that a folio holds several blocks. With a single block per folio there is no per-block state to get out of sync and the write is never lost.
Block devices only take the affected writeback path with CONFIG_BUFFER_HEAD=n.
Note: this commit is not yet upstream, but I was asked by Christoph Hellwig to submit this test (2).
(1): https://lore.kernel.org/linux-block/20260802-blkdev-fixes-v1-1-a82fc549fd74@columbia.edu/
(2): https://lore.kernel.org/linux-block/20260804140102.GA8894@lst.de/