Re: [PATCH] ext4: don't cache out of order extents

[Theodore Ts'o <tytso@mit.edu>]

On Thu, Oct 17, 2013 at 05:22:58PM +0200, Lukáš Czerner wrote:
> 
> I agree, since ext4_ext_check() should be really only used when
> reading data from disk. That said, we might actually remove the
> check from ext4_ext_precache() and ext4_ext_remove_space() because
> it seems to be that the check has already been done in ext4_iget()
> and it should be enough to check it once when reading from disk,
> right ?

Yes, since we do call ext4_ext_check() in ext4_iget() to verify the
root node of the extent tree located in ei->i_blocks[], it's strictly
speaking not necessary.  OTOH, there are at most four entries in
i_blocks[] that need to be verified, and it's a bit easier for the
contents of i_blocks[] to get corrupted by buggy code, so it's a
toss-up whether it's really worth it to remove it from those two
places, which aren't really hotspots.  It could be argued that are
plenty of other places that where we're not validating the root extent
tree node, so we might as well remove it from those two functions,
though.

> > Eryu's patch, or something like it, will still be needed so that in
> > the case of errors=countinue, we don't end up calling BUG_ON().
> 
> Hmm shouldn't we avoid that data in the case that it's corrupted
> rather than using it ? It seems like this is what the code would do
> anyway even with errors=continue when __ext4_ext_check() returns
> error.

Hmm, maybe we should set a flag indicating that the inode is bad, and
then cause attempts to read or write the contents of that inode should
return EIO.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html