| Message ID | 20211024033237.23662-1-wangyugui@e16-tech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix a check-integrity warning on write caching disabled disk | expand |
On Sun, Oct 24, 2021 at 4:40 AM wangyugui <wangyugui@e16-tech.com> wrote: > > xfstest/btrfs/220 tigger a check-integrity warning > btrfs: attempt to write superblock which references block which is not flushed > out of disk's write cache (block flush_gen=1, dev->flush_gen=0)! > WARNING: at fs/btrfs/check-integrity.c:2196 > btrfsic_process_written_superblock+0x22a/0x2a0 [btrfs] For stack traces it's best to paste the full version, not just the first line or two, and for readability you can skip the reformatting to make it fit within 75 character wide columns. This is on the borderline of subjectivity / personal preference, so take it only as a note for future submissions. Also a blank line before and after the stack trace helps with readability. > when > 1) CONFIG_BTRFS_FS_CHECK_INTEGRITY=y > 2) on a disk with WCE=0 > > 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. > > Signed-off-by: wangyugui <wangyugui@e16-tech.com> Looks good, thanks for doing it. Reviewed-by: Filipe Manana <fdmanana@suse.com> > --- > Changes since v1: > - update the changlog/code comment. (Filipe Manana) > - var(struct request_queue *q) is only needed when > !CONFIG_BTRFS_FS_CHECK_INTEGRITY > --- > fs/btrfs/disk-io.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 355ea88d5c5f..4ef06d0555b0 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3968,11 +3968,23 @@ static void btrfs_end_empty_barrier(struct bio *bio) > */ > 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 > + /* > + * 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. > + */ > + struct request_queue *q = bdev_get_queue(device->bdev); > if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) > return; > + #endif > > bio_reset(bio); > bio->bi_end_io = btrfs_end_empty_barrier; > -- > 2.32.0 >
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 355ea88d5c5f..4ef06d0555b0 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3968,11 +3968,23 @@ static void btrfs_end_empty_barrier(struct bio *bio) */ 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 + /* + * 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. + */ + struct request_queue *q = bdev_get_queue(device->bdev); if (!test_bit(QUEUE_FLAG_WC, &q->queue_flags)) return; + #endif bio_reset(bio); bio->bi_end_io = btrfs_end_empty_barrier;
xfstest/btrfs/220 tigger a check-integrity warning btrfs: attempt to write superblock which references block which is not flushed out of disk's write cache (block flush_gen=1, dev->flush_gen=0)! WARNING: at fs/btrfs/check-integrity.c:2196 btrfsic_process_written_superblock+0x22a/0x2a0 [btrfs] when 1) CONFIG_BTRFS_FS_CHECK_INTEGRITY=y 2) on a disk with WCE=0 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. Signed-off-by: wangyugui <wangyugui@e16-tech.com> --- Changes since v1: - update the changlog/code comment. (Filipe Manana) - var(struct request_queue *q) is only needed when !CONFIG_BTRFS_FS_CHECK_INTEGRITY --- fs/btrfs/disk-io.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)