On Wed, Jul 12, 2017 at 04:40:36PM +0200, David Sterba wrote: > On Tue, Jul 11, 2017 at 02:43:16PM -0600, Liu Bo wrote: > > When btrfs fails the checksum check, it'll fill the whole page with > > "1". > > One could ask, why is the page filled with 1s. Brought by commit > 07157aacb1ecd394a54949 from 2007, without mentioning any justification. > I'm more inclined to revisit this behaviour and drop it eventually. > > > However, if %csum_expected is 0 (which means there is no checksum), then > > for some unknown reason, we just pretend that the read is correct, so > > userspace would be confused about the dilemma that read is successful but > > getting a page with all content being "1". > > Here 'no checksum' means that no checksum was found but was expected, > right?
Yes, no checksum was found. > An EIO would fail the read, I don't see a reason why the page > needs to be "zeroed". The contents would be inaccessible anyway. > Right, resetting page's content is needed when we return 0 instead of -EIO. I guess it was introduced for testing. So yes, I'm glad to remove that part, will do in a v2. > > This can happen due to a bug in btrfs-convert. > > > > This fixes it by always returning errors if checksum doesn't match. > > Independent of the above, this fix makes sense. > > Reviewed-by: David Sterba <dste...@suse.com> Thank you for the comments. 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
