Re: [PATCH 4/5] quota: Move quota syncing to ->sync_fs method

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Wed, 2011-07-27 at 00:38 +0200, Jan Kara wrote:
> Since the moment writes to quota files are using block device page cache and
> space for quota structures is reserved at the moment they are first accessed we
> have no reason to sync quota before inode writeback. In fact this order is now
> only harmful since quota information can easily change during inode writeback
> (either because conversion of delayed-allocated extents or simply because of
> allocation of new blocks for simple filesystems not using page_mkwrite).
> 
> So move syncing of quota information after writeback of inodes into ->sync_fs
> method. This way we do not have to use ->quota_sync callback which is primarily
> intended for use by quotactl syscall anyway and we get rid of calling
> ->sync_fs() twice unnecessarily. We skip quota syncing for OCFS2 since it does
> proper quota journalling in all cases (unlike ext3, ext4, and reiserfs which
> also support legacy non-journalled quotas) and thus there are no dirty quota
> structures.
> 
> CC: "Theodore Ts'o" <tytso@mit.edu>
> CC: Steven Whitehouse <swhiteho@redhat.com>
> CC: Dave Kleikamp <shaggy@kernel.org>
> CC: Joel Becker <jlbec@evilplan.org>
> CC: reiserfs-devel@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.

> ---
>  fs/ext2/super.c     |    6 ++++++
>  fs/ext3/super.c     |    5 +++++
>  fs/ext4/super.c     |    5 +++++
>  fs/gfs2/super.c     |    2 ++
>  fs/jfs/super.c      |    5 +++++
>  fs/reiserfs/super.c |    5 +++++
>  fs/sync.c           |    3 ---
>  7 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index 1dd62ed..174782f 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -1180,6 +1180,12 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
>  	struct ext2_sb_info *sbi = EXT2_SB(sb);
>  	struct ext2_super_block *es = EXT2_SB(sb)->s_es;
>  
> +	/*
> +	 * Write quota structures to quota file, sync_blockdev() will write
> +	 * them to disk later
> +	 */
> +	dquot_writeback_dquots(sb, -1);
> +
>  	spin_lock(&sbi->s_lock);
>  	if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
>  		ext2_debug("setting valid to 0\n");
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index aad153e..a91869c 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -2507,6 +2507,11 @@ static int ext3_sync_fs(struct super_block *sb, int wait)
>  {
>  	tid_t target;
>  
> +	/*
> +	 * Writeback quota in non-journalled quota case - journalled quota has
> +	 * no dirty dquots
> +	 */
> +	dquot_writeback_dquots(sb, -1);
>  	if (journal_start_commit(EXT3_SB(sb)->s_journal, &target)) {
>  		if (wait)
>  			log_wait_commit(EXT3_SB(sb)->s_journal, target);
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 9ea71aa..c81ab5f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4199,6 +4199,11 @@ static int ext4_sync_fs(struct super_block *sb, int wait)
>  
>  	trace_ext4_sync_fs(sb, wait);
>  	flush_workqueue(sbi->dio_unwritten_wq);
> +	/*
> +	 * Writeback quota in non-journalled quota case - journalled quota has
> +	 * no dirty dquots
> +	 */
> +	dquot_writeback_dquots(sb, -1);
>  	if (jbd2_journal_start_commit(sbi->s_journal, &target)) {
>  		if (wait)
>  			jbd2_log_wait_commit(sbi->s_journal, target);
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index e361610..faf6e83 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -921,6 +921,8 @@ restart:
>  static int gfs2_sync_fs(struct super_block *sb, int wait)
>  {
>  	struct gfs2_sbd *sdp = sb->s_fs_info;
> +
> +	gfs2_quota_sync(sb, -1);
>  	if (wait && sdp)
>  		gfs2_log_flush(sdp, NULL);
>  	return 0;
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 06c8a67..a2c9cc1 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -603,6 +603,11 @@ static int jfs_sync_fs(struct super_block *sb, int wait)
>  
>  	/* log == NULL indicates read-only mount */
>  	if (log) {
> +		/*
> +		 * Write quota structures to quota file, sync_blockdev() will
> +		 * write them to disk later
> +		 */
> +		dquot_writeback_dquots(sb, -1);
>  		jfs_flush_journal(log, wait);
>  		jfs_syncpt(log, 0);
>  	}
> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index aa91089..a2bfc97 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -66,6 +66,11 @@ static int reiserfs_sync_fs(struct super_block *s, int wait)
>  {
>  	struct reiserfs_transaction_handle th;
>  
> +	/*
> +	 * Writeback quota in non-journalled quota case - journalled quota has
> +	 * no dirty dquots
> +	 */
> +	dquot_writeback_dquots(s, -1);
>  	reiserfs_write_lock(s);
>  	if (!journal_begin(&th, s, 1))
>  		if (!journal_end_sync(&th, s, 1))
> diff --git a/fs/sync.c b/fs/sync.c
> index 2b7d569..f07f991 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -30,9 +30,6 @@
>   */
>  static void __sync_filesystem(struct super_block *sb, int wait)
>  {
> -	if (sb->s_qcop && sb->s_qcop->quota_sync)
> -		sb->s_qcop->quota_sync(sb, -1);
> -
>  	if (wait)
>  		sync_inodes_sb(sb);
>  	else