Andreas Dilger wrote:
On Oct 18, 2006 16:56 -0500, Eric Sandeen wrote:
Andreas Dilger wrote:
The directory leaf data is kept in
the page cache and there is a helper function ext2_check_page() to mark
the page "checked". That means the page only needs to be checked once
after being read from disk, instead of each time through readdir.
ah, sure. Hm... well, this might be a bit of a performance hit if it's
checking cached data... let me think on that.
Well, having something like "ext3_dir_bread()" that verifies the leaf block
once if (!uptodate()) would be almost the same as ext2 with fairly little
effort. It would help performance in several places, at the slight risk
of not handling in-memory corruption after the block is read...
Right, I understand what you meant; I meant that adding the check as I had it
was extra overhead & a performance risk. I think missing in-memory corruption
is ok; if memory is getting corrupted then there are almost surely bigger
problems looming.
I'm not sure whether this is a win or not. It means that if there is ever
a directory with a bad leaf block any entries beyond that block are not
accessible anymore.
I'm amazed at how hard ext3 works to cope with bad blocks ;-)
It would fail all of your tests otherwise, right?
Well, this test basically just looks for oopses or hangs. If the filesystem
shut down at the first sign of trouble, that would satisfy this test.
That is one virtue of
ext2 having grown up in the days when bad blocks existed. Those days are
(sadly) coming back again, hence desire for fs-level checksums, etc.
*nod*
...
This obviously won't help if the whole inode is bogus, but then nothing
will catch all errors.
Yep, I'd thought maybe a size vs. blocks test might make sense; I think
there can never legitimately be a sparse directory?
Not currently, though there was some desire to allow this during htree
development, to allow shrinking large-but-empty directories.
Yep, I wondered about that. Any chance it'll happen?
Since this
already provokes an ext3_error() (which might be a panic()) to hit a hole
we can assume that this needs to be carefully implmemented.
Well, adding it as you suggested is in a case where it will -already- be calling
an ext3_error; adding the test just keeps it from going too much further after
that. I think it's safe, and a good idea.
I guess if the intent is to soldier on in the face of adversity, it
doesn't matter if it's an umappable offset or an IO error; ext3 wants to
go ahead & try the next one block anyway. So the size test probably
makes sense as a stopping point.
Well, it would also be possible to look into inode->i_blocks to see what
blocks exist past this offset, but that is complicated by the introduction
> <eom>
introduction of ...? :)
Your suggested test seems pretty sane to me.
-Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
