On Sat, Sep 16, 2017 at 01:58:34PM +0200, Goffredo Baroncelli wrote: > On 09/15/2017 11:06 PM, Liu Bo wrote: > > commit 4246a0b63bd8 ("block: add a bi_error field to struct bio") > > changed the logic of how dio read endio reports errors. > > > > For single stripe dio read, %bio->bi_status reflects the error before > > verifying checksum, and now we're updating it when data block matches > > with its checksum, while in the mismatching case, %bio->bi_status is > > not updated to relfect that. > > > > When some blocks in a file have been corrupted on disk, reading such a > > file ends up with > > > > 1) checksum errros are reported in kernel log > > 2) read(2) returns successfully with some content being 0x01. > > > > In order to fix it, we need to report its checksum mismatch error to > > the upper layer (dio layer in this case) as well. > > I tested it, and now it works: even using O_DIRECT -EIO is returned if the > file is corrupted. > > ghigo@venice:~/btrfs/crash-o-direct/t$ ls -li > total 16384 > 257 -rw-r--r-- 1 root root 16777216 Sep 15 20:51 abcd > ghigo@venice:~/btrfs/crash-o-direct/t$ date > Sat Sep 16 13:56:26 CEST 2017 > > ghigo@venice:~/btrfs/crash-o-direct/t$ cat abcd > cat: abcd: Input/output error > ghigo@venice:~/btrfs/crash-o-direct/t$ dmesg -T | tail -1 > [Sat Sep 16 13:56:29 2017] BTRFS warning (device sdd5): csum failed root 5 > ino 257 off 0 csum 0x98f94189 expected csum 0x0ab6be80 mirror 1 > > ghigo@venice:~/btrfs/crash-o-direct/t$ dd if=abcd iflag=direct > dd: error reading 'abcd': Input/output error > 0+0 records in > 0+0 records out > 0 bytes copied, 0.000404156 s, 0.0 kB/s > ghigo@venice:~/btrfs/crash-o-direct/t$ dmesg -T | tail -1 > [Sat Sep 16 13:56:41 2017] BTRFS warning (device sdd5): csum failed root 5 > ino 257 off 0 csum 0x98f94189 expected csum 0x0ab6be80 mirror 1 >
Thanks a lot, any chance I can get your 'Tested-by' tag? Thanks, -liubo > > > > > > > Signed-off-by: Liu Bo <bo.li....@oracle.com> > > Reported-by: Goffredo Baroncelli <kreij...@inwind.it> > > cc: Goffredo Baroncelli <kreij...@inwind.it> > > --- > > fs/btrfs/inode.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 24bcd5c..a46799e 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -8267,11 +8267,8 @@ static void btrfs_endio_direct_read(struct bio *bio) > > struct btrfs_io_bio *io_bio = btrfs_io_bio(bio); > > blk_status_t err = bio->bi_status; > > > > - if (dip->flags & BTRFS_DIO_ORIG_BIO_SUBMITTED) { > > + if (dip->flags & BTRFS_DIO_ORIG_BIO_SUBMITTED) > > err = btrfs_subio_endio_read(inode, io_bio, err); > > - if (!err) > > - bio->bi_status = 0; > > - } > > > > unlock_extent(&BTRFS_I(inode)->io_tree, dip->logical_offset, > > dip->logical_offset + dip->bytes - 1); > > @@ -8279,7 +8276,7 @@ static void btrfs_endio_direct_read(struct bio *bio) > > > > kfree(dip); > > > > - dio_bio->bi_status = bio->bi_status; > > + dio_bio->bi_status = err; > > dio_end_io(dio_bio); > > > > if (io_bio->end_io) > > > > > -- > gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> > Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html