On Fri, Sep 15, 2017 at 08:57:41PM +0200, Goffredo Baroncelli wrote: > On 09/15/2017 07:01 PM, Liu Bo wrote: > >> Conclusion: even if the file is corrupted and normally BTRFS prevent to > >> access it, using O_DIRECT > >> a) no error is returned to the caller > >> b) instead of the page stored on the disk, it is returned a page filled > >> with 0x01 (according also with the function __readpage_endio_check()) > >> > > We've queued a patch[1] to fix it, the behavior was introduced a long > > time ago and we guess it was for testing purpose. > > > > With the patch, you'll get -EIO from dio read, which is consistent > > with buffered read. > > I tried your patch, but it doesn't seem to address this issue. I still got > -EIO with a normal file access, and a page filled by 0x01 with O_DIRECT.
Oh, nice, thanks for trying it out. I see it now, the regression is actually introduced by a cleanup patch, commit 4246a0b63bd8f56a1469b12eafeb875b1041a451 block: add a bi_error field to struct bio It overrides our -EIO which should be returned by dio_end_io(). But I think we need further clarification on the correct behavior, ie. If a file got corrupted (wrong) data, both buffer'd read and O_DIRECT read will return -EIO to userspace, and that parcularly corrupted page will be reset with 0x01 for not exposing data to userspace. No exception should exist. > > If I understand your patch, this address the case where there is no any > checksum, which is not this case. The checksum exists and it is valid. It is > the data wrong. > Not for the case of nodatacsum, it's actually for the case where some checksum somehow got zero'd, and btrfs was not returning -EIO even if it got a mismatch. Anyway, your problem is surely a regression, thanks for reporting it. Thanks, -liubo -- 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