[PATCH 6/8] xfs: Use generic writers counter instead of m_active_trans counter

[Eric Sandeen]

On 1/24/12 2:05 AM, Dave Chinner wrote:
> On Fri, Jan 20, 2012 at 09:34:44PM +0100, Jan Kara wrote:
>> m_active_trans counter is racy wrt filesystem freezing. The patch replaces it
>> with generic counter of running transactions which is properly synchronized
>> with filesystem freezing. Things are a bit more complex because we need to log
>> a dummy transaction and free block counters after the filesystem is frozen so
>> we need to pass information to _xfs_trans_alloc() whether the transaction is
>> part of filesystem freezing or not.
>>
>> Signed-off-by: Jan Kara <jack at suse.cz>
>> ---
>>  fs/xfs/xfs_fsops.c |    5 +++--
>>  fs/xfs/xfs_fsops.h |    2 +-
>>  fs/xfs/xfs_iomap.c |    4 ++--
>>  fs/xfs/xfs_mount.c |    2 +-
>>  fs/xfs/xfs_mount.h |    2 --
>>  fs/xfs/xfs_super.c |    3 +--
>>  fs/xfs/xfs_sync.c  |   13 +++----------
>>  fs/xfs/xfs_trans.c |   19 ++++++++++++-------
>>  fs/xfs/xfs_trans.h |    3 ++-
>>  9 files changed, 25 insertions(+), 28 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
>> index 1c6fdeb..503fdfa 100644
>> --- a/fs/xfs/xfs_fsops.c
>> +++ b/fs/xfs/xfs_fsops.c
>> @@ -645,12 +645,13 @@ out:
>>   */
>>  int
>>  xfs_fs_log_dummy(
>> -	xfs_mount_t	*mp)
>> +	xfs_mount_t	*mp,
>> +	bool		for_freeze)
> 
> What does "for_freeze" mean? If it is true, does it mean we are in a
> freeze or not in a freeze? I can't really tell from the code,
> because it just passed true or false, and in one case the code
> always passes false even though the code can be called after
> SB_FREEZE_WRITE is set (xfs_quiesce_data() via sync_filesystem())

Is that a problem?  This is all about the FREEZE_TRANS context right?
So whether we are under FREEZE_WRITE (during freeze_super) or not
(during i.e. sys_sync) I think it's ok, no?  See more below...

>>  #endif	/* __XFS_FSOPS_H__ */
>> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
>> index 9afa282..fd47f6e 100644
>> --- a/fs/xfs/xfs_iomap.c
>> +++ b/fs/xfs/xfs_iomap.c
>> @@ -679,8 +679,8 @@ xfs_iomap_write_unwritten(
>>  		 * the same inode that we complete here and might deadlock
>>  		 * on the iolock.
>>  		 */
>> -		xfs_wait_for_freeze(mp, SB_FREEZE_TRANS);
>> -		tp = _xfs_trans_alloc(mp, XFS_TRANS_STRAT_WRITE, KM_NOFS);
>> +		tp = _xfs_trans_alloc(mp, XFS_TRANS_STRAT_WRITE,
>> +				KM_NOFS, false);
> 
> This is a documentation regression - the code was clearly self
> documenting w.r.t. frozen filesystem behaviour. It isn't anymore.
> 
> I'd suggest that we need:
> 
> #define XFS_WAIT_FOR_FREEZE	false
> #define XFS_IGNORE_FROZEN_SB	true