On Feb 28, 2007 19:46 +0300, Dmitriy Monakhov wrote:
> Where are many places where xxxx_journal_stop() return code wasn't
> checked. Off cause xxxx_journal_stop() failed very rarely (and usually
> with fatal consequences), but this does'n meen it should not be checked.
I fully agree with this. Otherwise an application will believe that the
operation has succeeded when in fact the filesystem has failed. It would
notice this on the next filesystem operation, but that might be too late
for e.g. an MTA that just confirmed acceptance of an email.
There are also several places in the code where we don't check error
returns from ext3_journal_get_write_access(). I have a patch that I
was starting to work on but never managed to finish/test. I'd be
happy if you had time to finish it off.
================= ext3-check-jbd-errors-2.6.20.patch =================
--- ./fs/ext3/namei.c.orig 2007-02-08 08:10:20.000000000 +0800
+++ ./fs/ext3/namei.c 2007-02-17 07:56:16.000000000 +0800
@@ -59,9 +59,14 @@ static struct buffer_head *ext3_append(h
*block = inode->i_size >> inode->i_sb->s_blocksize_bits;
if ((bh = ext3_bread(handle, inode, *block, 1, err))) {
- inode->i_size += inode->i_sb->s_blocksize;
- EXT3_I(inode)->i_disksize = inode->i_size;
- ext3_journal_get_write_access(handle,bh);
+ *err = ext3_journal_get_write_access(handle,bh);
+ if (err) {
+ brelse(bh);
+ bh = NULL;
+ } else {
+ inode->i_size += inode->i_sb->s_blocksize;
+ EXT3_I(inode)->i_disksize = inode->i_size;
+ }
}
return bh;
}
@@ -1597,8 +1602,12 @@ static int ext3_delete_entry (handle_t *
if (!ext3_check_dir_entry("ext3_delete_entry", dir, de, bh, i))
return -EIO;
if (de == de_del) {
+ int err;
+
BUFFER_TRACE(bh, "get_write_access");
- ext3_journal_get_write_access(handle, bh);
+ err = ext3_journal_get_write_access(handle, bh);
+ if (err)
+ return err;
if (pde)
pde->rec_len =
cpu_to_le16(le16_to_cpu(pde->rec_len) +
@@ -1758,7 +1767,13 @@ retry:
goto out_stop;
}
BUFFER_TRACE(dir_block, "get_write_access");
- ext3_journal_get_write_access(handle, dir_block);
+ err = ext3_journal_get_write_access(handle, dir_block);
+ if (err) {
+ drop_nlink(inode); /* is this nlink == 0? */
+ ext3_mark_inode_dirty(handle, inode);
+ iput (inode);
+ goto out_stop;
+ }
de = (struct ext3_dir_entry_2 *) dir_block->b_data;
de->inode = cpu_to_le32(inode->i_ino);
de->name_len = 1;
@@ -2262,6 +2277,11 @@ static int ext3_rename (struct inode * o
if (!new_inode) {
brelse (new_bh);
new_bh = NULL;
+ } else {
+ BUFFER_TRACE(new_bh, "get write access");
+ retval = ext3_journal_get_write_access(handle, new_bh);
+ if (retval)
+ goto end_rename;
}
}
if (S_ISDIR(old_inode->i_mode)) {
@@ -2280,14 +2300,16 @@ static int ext3_rename (struct inode * o
if (!new_inode && new_dir!=old_dir &&
new_dir->i_nlink >= EXT3_LINK_MAX)
goto end_rename;
+ BUFFER_TRACE(dir_bh, "get_write_access");
+ retval = ext3_journal_get_write_access(handle, dir_bh);
+ if (retval)
+ goto end_rename;
}
if (!new_bh) {
retval = ext3_add_entry (handle, new_dentry, old_inode);
if (retval)
goto end_rename;
} else {
- BUFFER_TRACE(new_bh, "get write access");
- ext3_journal_get_write_access(handle, new_bh);
new_de->inode = cpu_to_le32(old_inode->i_ino);
if (EXT3_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
EXT3_FEATURE_INCOMPAT_FILETYPE))
@@ -2341,8 +2363,6 @@ static int ext3_rename (struct inode * o
old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME_SEC;
ext3_update_dx_flag(old_dir);
if (dir_bh) {
- BUFFER_TRACE(dir_bh, "get_write_access");
- ext3_journal_get_write_access(handle, dir_bh);
PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
BUFFER_TRACE(dir_bh, "call ext3_journal_dirty_metadata");
ext3_journal_dirty_metadata(handle, dir_bh);
==============================================================================
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
-
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
