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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
