lkml.org 
[lkml]   [2004]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Ext2-devel] [CHECKER] warnings in fs/ext3/namei.c (2.4.19) where disk read errors get ignored, causing non-empty dir to be deleted
On May 03, 2004  16:10 +0200, Jörn Engel wrote:
> On Tue, 27 April 2004 01:44:55 -0600, Andreas Dilger wrote:
> > Again a conscious decision. If a name is potentially inaccessible
> > because of an IO error it is better to allow the creation of a
> > potentially duplicate name than refuse creation of any new entries
> > in the directory. It's a matter of allowing the filesystem to be
> > used as well as possible in the face of failures vs. just giving up
> > and refusing to do anything.
>
> Do you mind if I doubt the sanity of whoever made that decision? When
> my hard drive fails, I don't care about writing to the fs too much
> anymore, I want to *notice* the failure early and to *read* as much as
> possible, then put the drive on a pile for test hardware.

If that's what you want, then mount the filesystem with "errors=remount-ro"
and you will get it. You can even mount it with "errors=panic" so that the
node reboots and does a full fsck immediately. For users that have a few
bad blocks on their disk and can't afford to throw the whole disk away this
is a reasonable course of action.

Two things that do deserve fixing in these call paths are that
a) we don't call ext3_error() for an IO error in ext3_find_entry(), so we
won't do the normal ext3 error handling (mark SB in error, remount-ro
or panic if desired);
b) in empty_dir() we don't continue checking for non-empty blocks after a
content error (ext3_check_dir_entry() calls ext3_error() already);
c) we had decided not to mark the SB in error for holes in directories to
allow leway in the indexed-directory implementation, but this change
incorrectly also disabled marking the SB in error for real IO errors.

Patch applies equally well to 2.4.25 and 2.6-current.

Cheers, Andreas
========================== ext3-direrrors.diff =============================
--- ./fs/ext3/namei.c.orig 2004-03-09 16:46:43.000000000 -0700
+++ ./fs/ext3/namei.c 2004-05-03 10:10:53.000000000 -0600
@@ -825,6 +825,8 @@ restart:
wait_on_buffer(bh);
if (!buffer_uptodate(bh)) {
/* read error, skip block & hope for the best */
+ ext3_error(sb, __FUNCTION__, "reading directory #%lu "
+ "offset %lu\n", dir->i_ino, block);
brelse(bh);
goto next;
}
@@ -1712,14 +1720,19 @@ static int empty_dir (struct inode * ino
struct buffer_head * bh;
struct ext3_dir_entry_2 * de, * de1;
struct super_block * sb;
- int err;
+ int err = 0;

sb = inode->i_sb;
if (inode->i_size < EXT3_DIR_REC_LEN(1) + EXT3_DIR_REC_LEN(2) ||
!(bh = ext3_bread (NULL, inode, 0, 0, &err))) {
- ext3_warning (inode->i_sb, "empty_dir",
- "bad directory (dir #%lu) - no data block",
- inode->i_ino);
+ if (err)
+ ext3_error(inode->i_sb, __FUNCTION__,
+ "error %d reading directory #%lu offset 0",
+ err, inode->i_ino);
+ else
+ ext3_warning(inode->i_sb, __FUNCTION__,
+ "bad directory (dir #%lu) - no data block",
+ inode->i_ino);
return 1;
}
de = (struct ext3_dir_entry_2 *) bh->b_data;
@@ -1741,24 +1754,26 @@ static int empty_dir (struct inode * ino
while (offset < inode->i_size ) {
if (!bh ||
(void *) de >= (void *) (bh->b_data+sb->s_blocksize)) {
+ err = 0;
brelse (bh);
bh = ext3_bread (NULL, inode,
offset >> EXT3_BLOCK_SIZE_BITS(sb), 0, &err);
if (!bh) {
-#if 0
- ext3_error (sb, "empty_dir",
- "directory #%lu contains a hole at offset %lu",
- inode->i_ino, offset);
-#endif
+ if (err)
+ ext3_error(sb, __FUNCTION__,
+ "error %d reading directory"
+ " #%lu offset %lu",
+ err, inode->i_ino, offset);
offset += sb->s_blocksize;
continue;
}
de = (struct ext3_dir_entry_2 *) bh->b_data;
}
- if (!ext3_check_dir_entry ("empty_dir", inode, de, bh,
- offset)) {
- brelse (bh);
- return 1;
+ if (!ext3_check_dir_entry("empty_dir", inode, de, bh, offset)) {
+ de = (struct ext3_dir_entry_2 *)(bh->b_data +
+ sb->s_blocksize);
+ offset = (offset | (sb->s_blocksize - 1)) + 1;
+ continue;
}
if (le32_to_cpu(de->inode)) {
brelse (bh);
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:02    [from the cache]
©2003-2011 Jasper Spaans