Re: [PATCH v2] Adding support to freeze and unfreeze a journal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/08/2011 11:24 AM, Marco Stornelli wrote:

Thanks for your review!
Il 07/05/2011 22:04, Surbhi Palande ha scritto:
On freezing the F.S, the journal should be frozen as well. This
implies not
being able to start any new transactions on a frozen journal. Similarly,
thawing a F.S should thaw a journal and this should conversely start
accepting
new transactions. While the F.S is frozen any request to start a new
handle should end up on a wait queue till the F.S is thawed back again.

Adding support to freeze and thaw a journal and leveraging this
support in
freezing and thawing ext4.

Signed-off-by: Surbhi Palande<surbhi.palande@xxxxxxxxxxxxx>
---
Changes since v1:
* Check for the j_flag and if frozen release the j_state_lock before
sleeping
on wait queue
* Export the freeze, thaw routines
* Added a barrier after setting the j_flags = JBD2_FROZEN

fs/ext4/super.c | 20 ++++++--------------
fs/jbd2/journal.c | 1 +
fs/jbd2/transaction.c | 43 +++++++++++++++++++++++++++++++++++++++++++
include/linux/jbd2.h | 9 +++++++++
4 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8553dfb..796aa4c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4179,23 +4179,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;
}

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index e0ec3db..5e46333 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -842,6 +842,7 @@ static journal_t * journal_init_common (void)
init_waitqueue_head(&journal->j_wait_checkpoint);
init_waitqueue_head(&journal->j_wait_commit);
init_waitqueue_head(&journal->j_wait_updates);
+ init_waitqueue_head(&journal->j_wait_frozen);
mutex_init(&journal->j_barrier);
mutex_init(&journal->j_checkpoint_mutex);
spin_lock_init(&journal->j_revoke_lock);
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 05fa77a..3283c77 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -171,6 +171,17 @@ repeat:
journal->j_barrier_count == 0);
goto repeat;
}
+ /* dont let a new handle start when a journal is frozen.
+ * jbd2_journal_freeze calls jbd2_journal_unlock_updates() only after
+ * the jflags indicate that the journal is frozen. So if the
+ * j_barrier_count is 0, then check if this was made 0 by the freezing
+ * process
+ */
+ if (journal->j_flags& JBD2_FROZEN) {
+ read_unlock(&journal->j_state_lock);
+ jbd2_check_frozen(journal);
+ goto repeat;
+ }

if (!journal->j_running_transaction) {
read_unlock(&journal->j_state_lock);
@@ -489,6 +500,38 @@ int jbd2_journal_restart(handle_t *handle, int
nblocks)
}
EXPORT_SYMBOL(jbd2_journal_restart);

+int jbd2_journal_freeze(journal_t *journal)
+{
+ int error = 0;
+ /* Now we set up the journal barrier. */
+ jbd2_journal_lock_updates(journal);
+
+ /*
+ * Don't clear the needs_recovery flag if we failed to flush
+ * the journal.
+ */
+ error = jbd2_journal_flush(journal);
+ if (error>= 0) {
+ write_lock(&journal->j_state_lock);
+ journal->j_flags = journal->j_flags | JBD2_FROZEN;

Better journal->j_flags |= JBD2_FROZEN;

I was wondering why this is actually better than the longer form? Is there any technical reason other than preference of coding style here?


+ write_unlock(&journal->j_state_lock);
+ }
+ jbd2_journal_unlock_updates(journal);
+ return error;
+}
+EXPORT_SYMBOL(jbd2_journal_freeze);
+
+void jbd2_journal_thaw(journal_t * journal)
+{
+ write_lock(&journal->j_state_lock);
+ journal->j_flags = journal->j_flags&= ~JBD2_FROZEN;

What? Maybe journal->j_flags &= ~JBD2_FROZEN;

This definitely is a typo and needs a change. Again, why do you recommend the shorter form?


+ write_unlock(&journal->j_state_lock);
+ smp_wmb();

It'd be better to add a comment here for this barrier.
Ok!

+ wake_up(&journal->j_wait_frozen);
+}
+EXPORT_SYMBOL(jbd2_journal_thaw);
+
+

Marco
Warm Regards,
Surbhi.

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux