diff mbox

[v3] Adding support to freeze and unfreeze a journal

Message ID 1304952692-3858-1-git-send-email-surbhi.palande@canonical.com
State Superseded, archived
Headers show

Commit Message

Surbhi Palande May 9, 2011, 2:51 p.m. UTC
The journal should be frozen when a F.S freezes. What this means is that till
the F.S is thawed again, no new transactions should be accepted by the
journal. When the F.S thaws, inturn it should thaw the journal and this should
allow the journal to resume accepting new transactions.
While the F.S has frozen the journal, the clients of journal on calling
jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
up the sleeping clients and journalling can progress normally.

Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
---
Changes since the last patch:
* Changed to the shorter forms of expressions eg: x |= y
* removed the unnecessary barrier

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

Comments

Jan Kara May 9, 2011, 3:08 p.m. UTC | #1
On Mon 09-05-11 17:51:32, Surbhi Palande wrote:
> The journal should be frozen when a F.S freezes. What this means is that till
> the F.S is thawed again, no new transactions should be accepted by the
> journal. When the F.S thaws, inturn it should thaw the journal and this should
> allow the journal to resume accepting new transactions.
> While the F.S has frozen the journal, the clients of journal on calling
> jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
> up the sleeping clients and journalling can progress normally.
  The patch looks fine. I'd just add here a scheme of the race which can
happen if we don't really freeze the journal and rely on vfs... You can add:
Acked-by: Jan Kara <jack@suse.cz>

									Honza
> 
> Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
> ---
> Changes since the last patch:
> * Changed to the shorter forms of expressions eg: x |= y
> * removed the unnecessary barrier
> 
>  fs/ext4/super.c       |   20 ++++++--------------
>  fs/jbd2/journal.c     |    1 +
>  fs/jbd2/transaction.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/jbd2.h  |   10 ++++++++++
>  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..b040293 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,37 @@ 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 |= JBD2_FROZEN;
> +		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 &= ~JBD2_FROZEN;
> +	write_unlock(&journal->j_state_lock);
> +	wake_up(&journal->j_wait_frozen);
> +}
> +EXPORT_SYMBOL(jbd2_journal_thaw);
> +
> +
>  /**
>   * void jbd2_journal_lock_updates () - establish a transaction barrier.
>   * @journal:  Journal to establish a barrier on.
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index a32dcae..c7885b2 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -718,6 +718,7 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>   * @j_wait_checkpoint:  Wait queue to trigger checkpointing
>   * @j_wait_commit: Wait queue to trigger commit
>   * @j_wait_updates: Wait queue to wait for updates to complete
> + * @j_wait_frozen: Wait queue to wait for journal to thaw
>   * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
>   * @j_head: Journal head - identifies the first unused block in the journal
>   * @j_tail: Journal tail - identifies the oldest still-used block in the
> @@ -835,6 +836,9 @@ struct journal_s
>  	/* Wait queue to wait for updates to complete */
>  	wait_queue_head_t	j_wait_updates;
>  
> +	/* Wait queue to wait for journal to thaw*/
> +	wait_queue_head_t	j_wait_frozen;
> +
>  	/* Semaphore for locking against concurrent checkpoints */
>  	struct mutex		j_checkpoint_mutex;
>  
> @@ -1013,7 +1017,11 @@ struct journal_s
>  #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
>  						 * data write error in ordered
>  						 * mode */
> +#define JBD2_FROZEN	0x080   /* Journal thread is frozen as the filesystem is frozen */
> +
>  
> +#define jbd2_check_frozen(journal)	\
> +		wait_event(journal->j_wait_frozen, (journal->j_flags & JBD2_FROZEN))
>  /*
>   * Function declarations for the journaling transaction and buffer
>   * management
> @@ -1121,6 +1129,8 @@ extern void	 jbd2_journal_invalidatepage(journal_t *,
>  				struct page *, unsigned long);
>  extern int	 jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
>  extern int	 jbd2_journal_stop(handle_t *);
> +extern int	 jbd2_journal_freeze(journal_t *);
> +extern void	 jbd2_journal_thaw(journal_t *);
>  extern int	 jbd2_journal_flush (journal_t *);
>  extern void	 jbd2_journal_lock_updates (journal_t *);
>  extern void	 jbd2_journal_unlock_updates (journal_t *);
> -- 
> 1.7.1
>
Eric Sandeen May 9, 2011, 3:23 p.m. UTC | #2
On 5/9/11 9:51 AM, Surbhi Palande wrote:
> The journal should be frozen when a F.S freezes. What this means is that till
> the F.S is thawed again, no new transactions should be accepted by the
> journal. When the F.S thaws, inturn it should thaw the journal and this should
> allow the journal to resume accepting new transactions.
> While the F.S has frozen the journal, the clients of journal on calling
> jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
> up the sleeping clients and journalling can progress normally.

Can I ask how this was tested?

Ideally anything you found useful for testing should probably be integrated
into the xfstests test suite so that we don't regresss in the future.

thanks,
-Eric

> Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
> ---
> Changes since the last patch:
> * Changed to the shorter forms of expressions eg: x |= y
> * removed the unnecessary barrier
> 
>  fs/ext4/super.c       |   20 ++++++--------------
>  fs/jbd2/journal.c     |    1 +
>  fs/jbd2/transaction.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/jbd2.h  |   10 ++++++++++
>  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..b040293 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,37 @@ 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 |= JBD2_FROZEN;
> +		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 &= ~JBD2_FROZEN;
> +	write_unlock(&journal->j_state_lock);
> +	wake_up(&journal->j_wait_frozen);
> +}
> +EXPORT_SYMBOL(jbd2_journal_thaw);
> +
> +
>  /**
>   * void jbd2_journal_lock_updates () - establish a transaction barrier.
>   * @journal:  Journal to establish a barrier on.
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index a32dcae..c7885b2 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -718,6 +718,7 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>   * @j_wait_checkpoint:  Wait queue to trigger checkpointing
>   * @j_wait_commit: Wait queue to trigger commit
>   * @j_wait_updates: Wait queue to wait for updates to complete
> + * @j_wait_frozen: Wait queue to wait for journal to thaw
>   * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
>   * @j_head: Journal head - identifies the first unused block in the journal
>   * @j_tail: Journal tail - identifies the oldest still-used block in the
> @@ -835,6 +836,9 @@ struct journal_s
>  	/* Wait queue to wait for updates to complete */
>  	wait_queue_head_t	j_wait_updates;
>  
> +	/* Wait queue to wait for journal to thaw*/
> +	wait_queue_head_t	j_wait_frozen;
> +
>  	/* Semaphore for locking against concurrent checkpoints */
>  	struct mutex		j_checkpoint_mutex;
>  
> @@ -1013,7 +1017,11 @@ struct journal_s
>  #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
>  						 * data write error in ordered
>  						 * mode */
> +#define JBD2_FROZEN	0x080   /* Journal thread is frozen as the filesystem is frozen */
> +
>  
> +#define jbd2_check_frozen(journal)	\
> +		wait_event(journal->j_wait_frozen, (journal->j_flags & JBD2_FROZEN))
>  /*
>   * Function declarations for the journaling transaction and buffer
>   * management
> @@ -1121,6 +1129,8 @@ extern void	 jbd2_journal_invalidatepage(journal_t *,
>  				struct page *, unsigned long);
>  extern int	 jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
>  extern int	 jbd2_journal_stop(handle_t *);
> +extern int	 jbd2_journal_freeze(journal_t *);
> +extern void	 jbd2_journal_thaw(journal_t *);
>  extern int	 jbd2_journal_flush (journal_t *);
>  extern void	 jbd2_journal_lock_updates (journal_t *);
>  extern void	 jbd2_journal_unlock_updates (journal_t *);

--
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
Surbhi Palande May 11, 2011, 7:06 a.m. UTC | #3
Hi Eric,

On 05/09/2011 06:23 PM, Eric Sandeen wrote:
> On 5/9/11 9:51 AM, Surbhi Palande wrote:
>> The journal should be frozen when a F.S freezes. What this means is that till
>> the F.S is thawed again, no new transactions should be accepted by the
>> journal. When the F.S thaws, inturn it should thaw the journal and this should
>> allow the journal to resume accepting new transactions.
>> While the F.S has frozen the journal, the clients of journal on calling
>> jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
>> up the sleeping clients and journalling can progress normally.
>
> Can I ask how this was tested?

Yes! I did the following on an ext4 fs mount:
1. fsfreeze -f $MNT
2. dd if=/dev/zero of=$MNT/file count=10 bs=1024 &
3. sync
4. fsfreeze -u $MNT

If the dd blocks on the start_handle, then the page cache is clean and 
sync should have nothing to write and everything will work fine. But 
otherwise this should sequence should create a deadlock.

I have attempted to create a patch for xfs-test. Shall send it out as a 
reply to this email soon!

Warm Regards,
Surbhi.



>
> Ideally anything you found useful for testing should probably be integrated
> into the xfstests test suite so that we don't regresss in the future.
>
> thanks,
> -Eric
>
>> Signed-off-by: Surbhi Palande<surbhi.palande@canonical.com>
>> ---
>> Changes since the last patch:
>> * Changed to the shorter forms of expressions eg: x |= y
>> * removed the unnecessary barrier
>>
>>   fs/ext4/super.c       |   20 ++++++--------------
>>   fs/jbd2/journal.c     |    1 +
>>   fs/jbd2/transaction.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/jbd2.h  |   10 ++++++++++
>>   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..b040293 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,37 @@ 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 |= JBD2_FROZEN;
>> +		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&= ~JBD2_FROZEN;
>> +	write_unlock(&journal->j_state_lock);
>> +	wake_up(&journal->j_wait_frozen);
>> +}
>> +EXPORT_SYMBOL(jbd2_journal_thaw);
>> +
>> +
>>   /**
>>    * void jbd2_journal_lock_updates () - establish a transaction barrier.
>>    * @journal:  Journal to establish a barrier on.
>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>> index a32dcae..c7885b2 100644
>> --- a/include/linux/jbd2.h
>> +++ b/include/linux/jbd2.h
>> @@ -718,6 +718,7 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>>    * @j_wait_checkpoint:  Wait queue to trigger checkpointing
>>    * @j_wait_commit: Wait queue to trigger commit
>>    * @j_wait_updates: Wait queue to wait for updates to complete
>> + * @j_wait_frozen: Wait queue to wait for journal to thaw
>>    * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
>>    * @j_head: Journal head - identifies the first unused block in the journal
>>    * @j_tail: Journal tail - identifies the oldest still-used block in the
>> @@ -835,6 +836,9 @@ struct journal_s
>>   	/* Wait queue to wait for updates to complete */
>>   	wait_queue_head_t	j_wait_updates;
>>
>> +	/* Wait queue to wait for journal to thaw*/
>> +	wait_queue_head_t	j_wait_frozen;
>> +
>>   	/* Semaphore for locking against concurrent checkpoints */
>>   	struct mutex		j_checkpoint_mutex;
>>
>> @@ -1013,7 +1017,11 @@ struct journal_s
>>   #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
>>   						 * data write error in ordered
>>   						 * mode */
>> +#define JBD2_FROZEN	0x080   /* Journal thread is frozen as the filesystem is frozen */
>> +
>>
>> +#define jbd2_check_frozen(journal)	\
>> +		wait_event(journal->j_wait_frozen, (journal->j_flags&  JBD2_FROZEN))
>>   /*
>>    * Function declarations for the journaling transaction and buffer
>>    * management
>> @@ -1121,6 +1129,8 @@ extern void	 jbd2_journal_invalidatepage(journal_t *,
>>   				struct page *, unsigned long);
>>   extern int	 jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
>>   extern int	 jbd2_journal_stop(handle_t *);
>> +extern int	 jbd2_journal_freeze(journal_t *);
>> +extern void	 jbd2_journal_thaw(journal_t *);
>>   extern int	 jbd2_journal_flush (journal_t *);
>>   extern void	 jbd2_journal_lock_updates (journal_t *);
>>   extern void	 jbd2_journal_unlock_updates (journal_t *);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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
Andreas Dilger May 11, 2011, 9:05 a.m. UTC | #4
On 2011-05-11, at 1:06 AM, Surbhi Palande <surbhi.palande@canonical.com> wrote:

> On 05/09/2011 06:23 PM, Eric Sandeen wrote:
>> On 5/9/11 9:51 AM, Surbhi Palande wrote:
>>> The journal should be frozen when a F.S freezes. What this means is that till
>>> the F.S is thawed again, no new transactions should be accepted by the
>>> journal. When the F.S thaws, inturn it should thaw the journal and this should
>>> allow the journal to resume accepting new transactions.
>>> While the F.S has frozen the journal, the clients of journal on calling
>>> jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
>>> up the sleeping clients and journalling can progress normally.
>> 
>> Can I ask how this was tested?
> 
> Yes! I did the following on an ext4 fs mount:
> 1. fsfreeze -f $MNT
> 2. dd if=/dev/zero of=$MNT/file count=10 bs=1024 &
> 3. sync
> 4. fsfreeze -u $MNT
> 
> If the dd blocks on the start_handle, then the page cache is clean and sync should have nothing to write and everything will work fine. But otherwise this should sequence should create a deadlock.

Sorry to ask the obvious question, but presumably this test fails without your patch?  It isn't clear from your comment that this is the case. 

> I have attempted to create a patch for xfs-test. Shall send it out as a reply to this email soon!
> 
> Warm Regards,
> Surbhi.
> 
> 
> 
>> 
>> Ideally anything you found useful for testing should probably be integrated
>> into the xfstests test suite so that we don't regresss in the future.
>> 
>> thanks,
>> -Eric
>> 
>>> Signed-off-by: Surbhi Palande<surbhi.palande@canonical.com>
>>> ---
>>> Changes since the last patch:
>>> * Changed to the shorter forms of expressions eg: x |= y
>>> * removed the unnecessary barrier
>>> 
>>>  fs/ext4/super.c       |   20 ++++++--------------
>>>  fs/jbd2/journal.c     |    1 +
>>>  fs/jbd2/transaction.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/jbd2.h  |   10 ++++++++++
>>>  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..b040293 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,37 @@ 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 |= JBD2_FROZEN;
>>> +        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&= ~JBD2_FROZEN;
>>> +    write_unlock(&journal->j_state_lock);
>>> +    wake_up(&journal->j_wait_frozen);
>>> +}
>>> +EXPORT_SYMBOL(jbd2_journal_thaw);
>>> +
>>> +
>>>  /**
>>>   * void jbd2_journal_lock_updates () - establish a transaction barrier.
>>>   * @journal:  Journal to establish a barrier on.
>>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>>> index a32dcae..c7885b2 100644
>>> --- a/include/linux/jbd2.h
>>> +++ b/include/linux/jbd2.h
>>> @@ -718,6 +718,7 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>>>   * @j_wait_checkpoint:  Wait queue to trigger checkpointing
>>>   * @j_wait_commit: Wait queue to trigger commit
>>>   * @j_wait_updates: Wait queue to wait for updates to complete
>>> + * @j_wait_frozen: Wait queue to wait for journal to thaw
>>>   * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
>>>   * @j_head: Journal head - identifies the first unused block in the journal
>>>   * @j_tail: Journal tail - identifies the oldest still-used block in the
>>> @@ -835,6 +836,9 @@ struct journal_s
>>>      /* Wait queue to wait for updates to complete */
>>>      wait_queue_head_t    j_wait_updates;
>>> 
>>> +    /* Wait queue to wait for journal to thaw*/
>>> +    wait_queue_head_t    j_wait_frozen;
>>> +
>>>      /* Semaphore for locking against concurrent checkpoints */
>>>      struct mutex        j_checkpoint_mutex;
>>> 
>>> @@ -1013,7 +1017,11 @@ struct journal_s
>>>  #define JBD2_ABORT_ON_SYNCDATA_ERR    0x040    /* Abort the journal on file
>>>                           * data write error in ordered
>>>                           * mode */
>>> +#define JBD2_FROZEN    0x080   /* Journal thread is frozen as the filesystem is frozen */
>>> +
>>> 
>>> +#define jbd2_check_frozen(journal)    \
>>> +        wait_event(journal->j_wait_frozen, (journal->j_flags&  JBD2_FROZEN))
>>>  /*
>>>   * Function declarations for the journaling transaction and buffer
>>>   * management
>>> @@ -1121,6 +1129,8 @@ extern void     jbd2_journal_invalidatepage(journal_t *,
>>>                  struct page *, unsigned long);
>>>  extern int     jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
>>>  extern int     jbd2_journal_stop(handle_t *);
>>> +extern int     jbd2_journal_freeze(journal_t *);
>>> +extern void     jbd2_journal_thaw(journal_t *);
>>>  extern int     jbd2_journal_flush (journal_t *);
>>>  extern void     jbd2_journal_lock_updates (journal_t *);
>>>  extern void     jbd2_journal_unlock_updates (journal_t *);
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
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
Surbhi Palande May 12, 2011, 9:40 a.m. UTC | #5
On 05/11/2011 12:05 PM, Andreas Dilger wrote:
> On 2011-05-11, at 1:06 AM, Surbhi Palande<surbhi.palande@canonical.com>  wrote:
>
>> On 05/09/2011 06:23 PM, Eric Sandeen wrote:
>>> On 5/9/11 9:51 AM, Surbhi Palande wrote:
>>>> The journal should be frozen when a F.S freezes. What this means is that till
>>>> the F.S is thawed again, no new transactions should be accepted by the
>>>> journal. When the F.S thaws, inturn it should thaw the journal and this should
>>>> allow the journal to resume accepting new transactions.
>>>> While the F.S has frozen the journal, the clients of journal on calling
>>>> jbd2_journal_start() will sleep on a wait queue. Thawing the journal will wake
>>>> up the sleeping clients and journalling can progress normally.
>>>
>>> Can I ask how this was tested?
>>
>> Yes! I did the following on an ext4 fs mount:
>> 1. fsfreeze -f $MNT
>> 2. dd if=/dev/zero of=$MNT/file count=10 bs=1024&
>> 3. sync
>> 4. fsfreeze -u $MNT
>>
>> If the dd blocks on the start_handle, then the page cache is clean and sync should have nothing to write and everything will work fine. But otherwise this should sequence should create a deadlock.
>
> Sorry to ask the obvious question, but presumably this test fails without your patch?  It isn't clear from your comment that this is the case.

Actually since this is a race its very difficult to see this 
deterministically. The deadlock is apparently regulary seen by running 
iozone on multipath - when a path comes back to service.

I imagined that this could be simulated by running a lot of I/O in the 
background and trying fsfreeze, unfreeze parallely (multiple times). 
Unfortunately its not easy to hit the bug - its never deterministic. I 
really smoke tested my patch using this method

{
	dd i/o in a loop (10000 times) & (background process)
	touch some file  & in the same loop (background process)
	
}

{
	freeze &, sync &, unfreeze &, sleep in a loop & (100 times)
        (background processes)
}

and saw that there was not any deadlock in this case.  But I have not 
tested/seen the deadlock with this script without the patch.

Sorry for not being clear before :(

Warm Regards,
Surbhi.















>
>> I have attempted to create a patch for xfs-test. Shall send it out as a reply to this email soon!
>>
>> Warm Regards,
>> Surbhi.
>>
>>
>>
>>>
>>> Ideally anything you found useful for testing should probably be integrated
>>> into the xfstests test suite so that we don't regresss in the future.
>>>
>>> thanks,
>>> -Eric
>>>
>>>> Signed-off-by: Surbhi Palande<surbhi.palande@canonical.com>
>>>> ---
>>>> Changes since the last patch:
>>>> * Changed to the shorter forms of expressions eg: x |= y
>>>> * removed the unnecessary barrier
>>>>
>>>>   fs/ext4/super.c       |   20 ++++++--------------
>>>>   fs/jbd2/journal.c     |    1 +
>>>>   fs/jbd2/transaction.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>>>   include/linux/jbd2.h  |   10 ++++++++++
>>>>   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..b040293 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,37 @@ 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 |= JBD2_FROZEN;
>>>> +        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&= ~JBD2_FROZEN;
>>>> +    write_unlock(&journal->j_state_lock);
>>>> +    wake_up(&journal->j_wait_frozen);
>>>> +}
>>>> +EXPORT_SYMBOL(jbd2_journal_thaw);
>>>> +
>>>> +
>>>>   /**
>>>>    * void jbd2_journal_lock_updates () - establish a transaction barrier.
>>>>    * @journal:  Journal to establish a barrier on.
>>>> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
>>>> index a32dcae..c7885b2 100644
>>>> --- a/include/linux/jbd2.h
>>>> +++ b/include/linux/jbd2.h
>>>> @@ -718,6 +718,7 @@ jbd2_time_diff(unsigned long start, unsigned long end)
>>>>    * @j_wait_checkpoint:  Wait queue to trigger checkpointing
>>>>    * @j_wait_commit: Wait queue to trigger commit
>>>>    * @j_wait_updates: Wait queue to wait for updates to complete
>>>> + * @j_wait_frozen: Wait queue to wait for journal to thaw
>>>>    * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
>>>>    * @j_head: Journal head - identifies the first unused block in the journal
>>>>    * @j_tail: Journal tail - identifies the oldest still-used block in the
>>>> @@ -835,6 +836,9 @@ struct journal_s
>>>>       /* Wait queue to wait for updates to complete */
>>>>       wait_queue_head_t    j_wait_updates;
>>>>
>>>> +    /* Wait queue to wait for journal to thaw*/
>>>> +    wait_queue_head_t    j_wait_frozen;
>>>> +
>>>>       /* Semaphore for locking against concurrent checkpoints */
>>>>       struct mutex        j_checkpoint_mutex;
>>>>
>>>> @@ -1013,7 +1017,11 @@ struct journal_s
>>>>   #define JBD2_ABORT_ON_SYNCDATA_ERR    0x040    /* Abort the journal on file
>>>>                            * data write error in ordered
>>>>                            * mode */
>>>> +#define JBD2_FROZEN    0x080   /* Journal thread is frozen as the filesystem is frozen */
>>>> +
>>>>
>>>> +#define jbd2_check_frozen(journal)    \
>>>> +        wait_event(journal->j_wait_frozen, (journal->j_flags&   JBD2_FROZEN))
>>>>   /*
>>>>    * Function declarations for the journaling transaction and buffer
>>>>    * management
>>>> @@ -1121,6 +1129,8 @@ extern void     jbd2_journal_invalidatepage(journal_t *,
>>>>                   struct page *, unsigned long);
>>>>   extern int     jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
>>>>   extern int     jbd2_journal_stop(handle_t *);
>>>> +extern int     jbd2_journal_freeze(journal_t *);
>>>> +extern void     jbd2_journal_thaw(journal_t *);
>>>>   extern int     jbd2_journal_flush (journal_t *);
>>>>   extern void     jbd2_journal_lock_updates (journal_t *);
>>>>   extern void     jbd2_journal_unlock_updates (journal_t *);
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

--
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
diff mbox

Patch

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..b040293 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,37 @@  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 |= JBD2_FROZEN;
+		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 &= ~JBD2_FROZEN;
+	write_unlock(&journal->j_state_lock);
+	wake_up(&journal->j_wait_frozen);
+}
+EXPORT_SYMBOL(jbd2_journal_thaw);
+
+
 /**
  * void jbd2_journal_lock_updates () - establish a transaction barrier.
  * @journal:  Journal to establish a barrier on.
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index a32dcae..c7885b2 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -718,6 +718,7 @@  jbd2_time_diff(unsigned long start, unsigned long end)
  * @j_wait_checkpoint:  Wait queue to trigger checkpointing
  * @j_wait_commit: Wait queue to trigger commit
  * @j_wait_updates: Wait queue to wait for updates to complete
+ * @j_wait_frozen: Wait queue to wait for journal to thaw
  * @j_checkpoint_mutex: Mutex for locking against concurrent checkpoints
  * @j_head: Journal head - identifies the first unused block in the journal
  * @j_tail: Journal tail - identifies the oldest still-used block in the
@@ -835,6 +836,9 @@  struct journal_s
 	/* Wait queue to wait for updates to complete */
 	wait_queue_head_t	j_wait_updates;
 
+	/* Wait queue to wait for journal to thaw*/
+	wait_queue_head_t	j_wait_frozen;
+
 	/* Semaphore for locking against concurrent checkpoints */
 	struct mutex		j_checkpoint_mutex;
 
@@ -1013,7 +1017,11 @@  struct journal_s
 #define JBD2_ABORT_ON_SYNCDATA_ERR	0x040	/* Abort the journal on file
 						 * data write error in ordered
 						 * mode */
+#define JBD2_FROZEN	0x080   /* Journal thread is frozen as the filesystem is frozen */
+
 
+#define jbd2_check_frozen(journal)	\
+		wait_event(journal->j_wait_frozen, (journal->j_flags & JBD2_FROZEN))
 /*
  * Function declarations for the journaling transaction and buffer
  * management
@@ -1121,6 +1129,8 @@  extern void	 jbd2_journal_invalidatepage(journal_t *,
 				struct page *, unsigned long);
 extern int	 jbd2_journal_try_to_free_buffers(journal_t *, struct page *, gfp_t);
 extern int	 jbd2_journal_stop(handle_t *);
+extern int	 jbd2_journal_freeze(journal_t *);
+extern void	 jbd2_journal_thaw(journal_t *);
 extern int	 jbd2_journal_flush (journal_t *);
 extern void	 jbd2_journal_lock_updates (journal_t *);
 extern void	 jbd2_journal_unlock_updates (journal_t *);