Re: [PATCH] btrfs: fix CHECK_INTEGRITY warning when !QUEUE_FLAG_WC
From: Filipe Manana <hidden>
Date: 2021-10-22 14:43:18
On Fri, Oct 22, 2021 at 9:29 AM wangyugui [off-list ref] wrote:
xfstest/btrfs/220 tigger a check-integrity warning when 1) CONFIG_BTRFS_FS_CHECK_INTEGRITY=y 2) xfstest/btrfs/220 run on a disk with WCE=1 In write_dev_flush(), submit_bio(REQ_SYNC | REQ_PREFLUSH) can be skipped when !QUEUE_FLAG_WC. but btrfsic_submit_bio() != submit_bio() when CONFIG_BTRFS_FS_CHECK_INTEGRITY
The change log is a bit confusing, something more detailed like this would be better: When a disk has write caching disabled, we skip submission of a bio with flush and sync requests before writing the superblock, since it's not needed. However when the integrity checker is enabled, this results in reports that there are metadata blocks referred by a superblock that were not properly flushed. So don't skip the bio submission only when the integrity checker is enabled for the sake of simplicity, since this is a debug tool and not meant for use in non-debug builds. Btw, the line length of a changelog should not exceed 75 characters. For reference, you could also paste the warning you got in the change log, it makes it easier to grep for fixes for example.
quoted hunk ↗ jump to hunk
Signed-off-by: wangyugui <redacted> --- fs/btrfs/disk-io.c | 6 ++++++ 1 file changed, 6 insertions(+)diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 355ea88..7b17357 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c@@ -3971,8 +3971,14 @@ static void write_dev_flush(struct btrfs_device *device) struct request_queue *q = bdev_get_queue(device->bdev); struct bio *bio = device->flush_bio; + #ifndef CONFIG_BTRFS_FS_CHECK_INTEGRITY + /* + * submit_bio(REQ_SYNC | REQ_PREFLUSH) can be skipped when !QUEUE_FLAG_WC. + * but btrfsic_submit_bio() != submit_bio() when CONFIG_BTRFS_FS_CHECK_INTEGRITY + */
The comment could use the same type of improvement as well.
if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags))
return;
+ #endifAs for the code itself, it looks good. Thanks.
bio_reset(bio);
bio->bi_end_io = btrfs_end_empty_barrier;
--
2.32.0-- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”