On Mon, Dec 05, 2011 at 12:54:45PM -0800, Kamal Mostafa wrote:
> From: Surbhi Palande <surbhi.palande@xxxxxxxxxxxxx>
> @@ -4330,23 +4330,15 @@ static int ext4_freeze(struct super_block *sb)
>
> journal = EXT4_SB(sb)->s_journal;
>
> - /* Now we set up the journal barrier. */
> - jbd2_journal_lock_updates(journal);
> -
> + error = jbd2_journal_freeze(journal);
> /*
> - * Don't clear the needs_recovery flag if we failed to flush
> + * Don't clear the needs_recovery flag if we failed to freeze
> * the journal.
> */
> - error = jbd2_journal_flush(journal);
> - if (error < 0)
> - goto out;
> -
> - /* Journal blocked and flushed, clear needs_recovery flag. */
> - EXT4_CLEAR_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER);
> - error = ext4_commit_super(sb, 1);
> -out:
> - /* we rely on s_frozen to stop further updates */
> - jbd2_journal_unlock_updates(EXT4_SB(sb)->s_journal);
> + if (error >= 0) {
> + EXT4_CLEAR_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER);
> + error = ext4_commit_super(sb, 1);
> + }
> return error;
> }
A strange goto-avoidance fetish here? Why not stick to the same style
as the function already had:
error = jbd2_journal_freeze(journal);
if (error)
goto out;
EXT4_CLEAR_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER);
error = ext4_commit_super(sb, 1);
out:
return error;
Easier to read the patch, easier to add new potential error cases to this
function later.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
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
