From: Andreas Dilger <adilger@clusterfs.com>
To: Dmitriy Monakhov <dmonakhov@sw.ru>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH][RFC] ext3: Handle ext[34]_journal_stop() failure
Date: Fri, 2 Mar 2007 17:06:16 +0800 [thread overview]
Message-ID: <20070302090616.GO6573@schatzie.adilger.int> (raw)
In-Reply-To: <871wka2pt4.fsf@sw.ru>
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.
prev parent reply other threads:[~2007-03-02 9:06 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-28 16:46 [PATCH][RFC] ext3: Handle ext[34]_journal_stop() failure Dmitriy Monakhov
2007-03-02 9:06 ` Andreas Dilger [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070302090616.GO6573@schatzie.adilger.int \
--to=adilger@clusterfs.com \
--cc=dmonakhov@sw.ru \
--cc=linux-ext4@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).