* [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() @ 2010-11-19 7:28 Namhyung Kim 2010-11-19 7:28 ` [PATCH 2/2] ext3: Add journal error check into ext3_rename() Namhyung Kim 2010-11-22 17:55 ` [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Jan Kara 0 siblings, 2 replies; 5+ messages in thread From: Namhyung Kim @ 2010-11-19 7:28 UTC (permalink / raw) To: Jan Kara, Andrew Morton, Andreas Dilger; +Cc: linux-ext4, linux-kernel Check return value of ext3_journal_get_write_access() and ext3_journal_dirty_metadata(). Signed-off-by: Namhyung Kim <namhyung@gmail.com> --- fs/ext3/namei.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c index 03fccc5..672cea1 100644 --- a/fs/ext3/namei.c +++ b/fs/ext3/namei.c @@ -1644,8 +1644,13 @@ static int ext3_delete_entry (handle_t *handle, 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) + goto journal_error; + if (pde) pde->rec_len = ext3_rec_len_to_disk( ext3_rec_len_from_disk(pde->rec_len) + @@ -1654,7 +1659,12 @@ static int ext3_delete_entry (handle_t *handle, de->inode = 0; dir->i_version++; BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata"); - ext3_journal_dirty_metadata(handle, bh); + err = ext3_journal_dirty_metadata(handle, bh); + if (err) { +journal_error: + ext3_std_error(dir->i_sb, err); + return err; + } return 0; } i += ext3_rec_len_from_disk(de->rec_len); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] ext3: Add journal error check into ext3_rename() 2010-11-19 7:28 [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Namhyung Kim @ 2010-11-19 7:28 ` Namhyung Kim 2010-11-22 18:05 ` Jan Kara 2010-11-22 17:55 ` [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Jan Kara 1 sibling, 1 reply; 5+ messages in thread From: Namhyung Kim @ 2010-11-19 7:28 UTC (permalink / raw) To: Jan Kara, Andrew Morton, Andreas Dilger; +Cc: linux-ext4, linux-kernel Check return value of ext3_journal_get_write_access() and ext3_journal_dirty_metadata(). 'new_bh' should be kept in order to get revoked in case of journal error in dir_bh. Signed-off-by: Namhyung Kim <namhyung@gmail.com> --- fs/ext3/namei.c | 27 +++++++++++++++++++++------ 1 files changed, 21 insertions(+), 6 deletions(-) diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c index 672cea1..8061281 100644 --- a/fs/ext3/namei.c +++ b/fs/ext3/namei.c @@ -2371,7 +2371,9 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, goto end_rename; } else { BUFFER_TRACE(new_bh, "get write access"); - ext3_journal_get_write_access(handle, new_bh); + retval = ext3_journal_get_write_access(handle, new_bh); + if (retval) + goto journal_error1; new_de->inode = cpu_to_le32(old_inode->i_ino); if (EXT3_HAS_INCOMPAT_FEATURE(new_dir->i_sb, EXT3_FEATURE_INCOMPAT_FILETYPE)) @@ -2380,9 +2382,12 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, new_dir->i_ctime = new_dir->i_mtime = CURRENT_TIME_SEC; ext3_mark_inode_dirty(handle, new_dir); BUFFER_TRACE(new_bh, "call ext3_journal_dirty_metadata"); - ext3_journal_dirty_metadata(handle, new_bh); - brelse(new_bh); - new_bh = NULL; + retval = ext3_journal_dirty_metadata(handle, new_bh); + if (retval) { +journal_error1: + ext3_std_error(new_dir->i_sb, retval); + goto end_rename; + } } /* @@ -2429,10 +2434,20 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, ext3_update_dx_flag(old_dir); if (dir_bh) { BUFFER_TRACE(dir_bh, "get_write_access"); - ext3_journal_get_write_access(handle, dir_bh); + retval = ext3_journal_get_write_access(handle, dir_bh); + if (retval) + goto journal_error2; 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); + retval = ext3_journal_dirty_metadata(handle, dir_bh); + if (retval) { +journal_error2: + if (new_bh) + ext3_journal_revoke(handle, new_bh->b_blocknr, + new_bh); + ext3_std_error(new_dir->i_sb, retval); + goto end_rename; + } drop_nlink(old_dir); if (new_inode) { drop_nlink(new_inode); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ext3: Add journal error check into ext3_rename() 2010-11-19 7:28 ` [PATCH 2/2] ext3: Add journal error check into ext3_rename() Namhyung Kim @ 2010-11-22 18:05 ` Jan Kara 2010-11-23 3:37 ` Namhyung Kim 0 siblings, 1 reply; 5+ messages in thread From: Jan Kara @ 2010-11-22 18:05 UTC (permalink / raw) To: Namhyung Kim Cc: Jan Kara, Andrew Morton, Andreas Dilger, linux-ext4, linux-kernel On Fri 19-11-10 16:28:36, Namhyung Kim wrote: > Check return value of ext3_journal_get_write_access() and > ext3_journal_dirty_metadata(). 'new_bh' should be kept in > order to get revoked in case of journal error in dir_bh. > > Signed-off-by: Namhyung Kim <namhyung@gmail.com> > --- > fs/ext3/namei.c | 27 +++++++++++++++++++++------ > 1 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c > index 672cea1..8061281 100644 > --- a/fs/ext3/namei.c > +++ b/fs/ext3/namei.c > @@ -2371,7 +2371,9 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, > goto end_rename; > } else { > BUFFER_TRACE(new_bh, "get write access"); > - ext3_journal_get_write_access(handle, new_bh); > + retval = ext3_journal_get_write_access(handle, new_bh); > + if (retval) > + goto journal_error1; > new_de->inode = cpu_to_le32(old_inode->i_ino); > if (EXT3_HAS_INCOMPAT_FEATURE(new_dir->i_sb, > EXT3_FEATURE_INCOMPAT_FILETYPE)) > @@ -2380,9 +2382,12 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, > new_dir->i_ctime = new_dir->i_mtime = CURRENT_TIME_SEC; > ext3_mark_inode_dirty(handle, new_dir); > BUFFER_TRACE(new_bh, "call ext3_journal_dirty_metadata"); > - ext3_journal_dirty_metadata(handle, new_bh); > - brelse(new_bh); > - new_bh = NULL; > + retval = ext3_journal_dirty_metadata(handle, new_bh); > + if (retval) { > +journal_error1: > + ext3_std_error(new_dir->i_sb, retval); > + goto end_rename; > + } > } > > /* > @@ -2429,10 +2434,20 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, > ext3_update_dx_flag(old_dir); > if (dir_bh) { > BUFFER_TRACE(dir_bh, "get_write_access"); > - ext3_journal_get_write_access(handle, dir_bh); > + retval = ext3_journal_get_write_access(handle, dir_bh); > + if (retval) > + goto journal_error2; > 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); > + retval = ext3_journal_dirty_metadata(handle, dir_bh); > + if (retval) { > +journal_error2: > + if (new_bh) > + ext3_journal_revoke(handle, new_bh->b_blocknr, > + new_bh); Uhuh, why ext3_journal_revoke()? I expect you want to cancel the changes you possibly did to new_bh. ext3_journal_forget() is for that but even that doesn't necessarily do what you want because it could cancel also changes some unrelated operation did to the buffer. So the only way to really undo the change is to set new_de->inode and new_de->file_type to original values. Also since we already unlinked the inode from the old directory, I'm not sure it's even beneficial to undo linking it to the new one. So I'd just bail out as fast as we can and leave on fsck to handle the mess... Honza > + ext3_std_error(new_dir->i_sb, retval); > + goto end_rename; > + } > drop_nlink(old_dir); > if (new_inode) { > drop_nlink(new_inode); > -- > 1.7.0.4 > -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ext3: Add journal error check into ext3_rename() 2010-11-22 18:05 ` Jan Kara @ 2010-11-23 3:37 ` Namhyung Kim 0 siblings, 0 replies; 5+ messages in thread From: Namhyung Kim @ 2010-11-23 3:37 UTC (permalink / raw) To: Jan Kara; +Cc: Andrew Morton, Andreas Dilger, linux-ext4, linux-kernel 2010-11-22 (월), 19:05 +0100, Jan Kara: > Uhuh, why ext3_journal_revoke()? I expect you want to cancel the changes > you possibly did to new_bh. ext3_journal_forget() is for that but even that > doesn't necessarily do what you want because it could cancel also changes > some unrelated operation did to the buffer. So the only way to really undo > the change is to set new_de->inode and new_de->file_type to original > values. Also since we already unlinked the inode from the old directory, > I'm not sure it's even beneficial to undo linking it to the new one. So I'd > just bail out as fast as we can and leave on fsck to handle the mess... > > Honza Right. I wanted cancel the changes but I wasn't sure what I did. Thanks for the explanation. I'll send v2 soon. -- Regards, Namhyung Kim -- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() 2010-11-19 7:28 [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Namhyung Kim 2010-11-19 7:28 ` [PATCH 2/2] ext3: Add journal error check into ext3_rename() Namhyung Kim @ 2010-11-22 17:55 ` Jan Kara 1 sibling, 0 replies; 5+ messages in thread From: Jan Kara @ 2010-11-22 17:55 UTC (permalink / raw) To: Namhyung Kim Cc: Jan Kara, Andrew Morton, Andreas Dilger, linux-ext4, linux-kernel On Fri 19-11-10 16:28:35, Namhyung Kim wrote: > Check return value of ext3_journal_get_write_access() and > ext3_journal_dirty_metadata(). Thanks, merged. Honza > > Signed-off-by: Namhyung Kim <namhyung@gmail.com> > --- > fs/ext3/namei.c | 14 ++++++++++++-- > 1 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c > index 03fccc5..672cea1 100644 > --- a/fs/ext3/namei.c > +++ b/fs/ext3/namei.c > @@ -1644,8 +1644,13 @@ static int ext3_delete_entry (handle_t *handle, > 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) > + goto journal_error; > + > if (pde) > pde->rec_len = ext3_rec_len_to_disk( > ext3_rec_len_from_disk(pde->rec_len) + > @@ -1654,7 +1659,12 @@ static int ext3_delete_entry (handle_t *handle, > de->inode = 0; > dir->i_version++; > BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata"); > - ext3_journal_dirty_metadata(handle, bh); > + err = ext3_journal_dirty_metadata(handle, bh); > + if (err) { > +journal_error: > + ext3_std_error(dir->i_sb, err); > + return err; > + } > return 0; > } > i += ext3_rec_len_from_disk(de->rec_len); > -- > 1.7.0.4 > -- Jan Kara <jack@suse.cz> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-23 3:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-19 7:28 [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Namhyung Kim 2010-11-19 7:28 ` [PATCH 2/2] ext3: Add journal error check into ext3_rename() Namhyung Kim 2010-11-22 18:05 ` Jan Kara 2010-11-23 3:37 ` Namhyung Kim 2010-11-22 17:55 ` [PATCH 1/2] ext3: Add journal error check into ext3_delete_entry() Jan Kara
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).