[PATCH] quota: cleanup dquota sync functions

[PATCH] quota: cleanup dquota sync functions

[Christoph Hellwig]

Currently the VFS calls vfs_dq_sync to sync out disk quotas for a given
superblock.  This is a small wrapper around sync_dquots which for the
case of a non-NULL superblock is a small wrapper around quota_sync_sb.

Just make quota_sync_sb global and call it directly.  Also call it
directly for those cases in quota.c that have a superblock and leave
sync_dquots purely an iterator over quota_sync_sb and remove it's
superblock argument.

To make this nicer move the check for the lack of a quota_sync method
from the callers into quota_sync_sb.

Btw, I think calling quota_sync_sb for the !wait case in __fsync_super
might be a bad idea because it calls into ->sync_fs with wait = 1.

Note that the patch requires Jan's sync rewrite applies first.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/quota/quota.c
===================================================================
--- linux-2.6.orig/fs/quota/quota.c	2009-04-25 19:53:49.074949184 +0200
+++ linux-2.6/fs/quota/quota.c	2009-04-25 20:11:23.598952601 +0200
@@ -159,10 +159,13 @@ static int check_quotactl_valid(struct s
 	return error;
 }
 
-static void quota_sync_sb(struct super_block *sb, int type)
+void quota_sync_sb(struct super_block *sb, int type)
 {
 	int cnt;
 
+	if (!sb->s_qcop->quota_sync)
+		return;
+
 	sb->s_qcop->quota_sync(sb, type);
 
 	if (sb_dqopt(sb)->flags & DQUOT_QUOTA_SYS_FILE)
@@ -192,16 +195,11 @@ static void quota_sync_sb(struct super_b
 	mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex);
 }
 
-void sync_dquots(struct super_block *sb, int type)
+static void sync_dquots(int type)
 {
+	struct super_block *sb;
 	int cnt;
 
-	if (sb) {
-		if (sb->s_qcop->quota_sync)
-			quota_sync_sb(sb, type);
-		return;
-	}
-
 	spin_lock(&sb_lock);
 restart:
 	list_for_each_entry(sb, &super_blocks, s_list) {
@@ -222,7 +220,7 @@ restart:
 		sb->s_count++;
 		spin_unlock(&sb_lock);
 		down_read(&sb->s_umount);
-		if (sb->s_root && sb->s_qcop->quota_sync)
+		if (sb->s_root)
 			quota_sync_sb(sb, type);
 		up_read(&sb->s_umount);
 		spin_lock(&sb_lock);
@@ -301,7 +299,10 @@ static int do_quotactl(struct super_bloc
 			return sb->s_qcop->set_dqblk(sb, type, id, &idq);
 		}
 		case Q_SYNC:
-			sync_dquots(sb, type);
+			if (sb)
+				quota_sync_sb(sb, type);
+			else
+				sync_dquots(type);
 			return 0;
 
 		case Q_XQUOTAON:
Index: linux-2.6/fs/super.c
===================================================================
--- linux-2.6.orig/fs/super.c	2009-04-25 19:54:15.376074155 +0200
+++ linux-2.6/fs/super.c	2009-04-25 19:54:16.125949480 +0200
@@ -259,7 +259,7 @@ EXPORT_SYMBOL(unlock_super);
 
 static int __fsync_super(struct super_block *sb, int wait)
 {
-	vfs_dq_sync(sb);
+	quota_sync_sb(sb, -1);
 	sync_inodes_sb(sb, wait);
 	lock_super(sb);
 	if (sb->s_dirt && sb->s_op->write_super)
Index: linux-2.6/include/linux/quotaops.h
===================================================================
--- linux-2.6.orig/include/linux/quotaops.h	2009-04-25 19:53:49.086949524 +0200
+++ linux-2.6/include/linux/quotaops.h	2009-04-25 19:54:16.126949327 +0200
@@ -20,7 +20,7 @@ static inline struct quota_info *sb_dqop
 /*
  * declaration of quota_function calls in kernel.
  */
-void sync_dquots(struct super_block *sb, int type);
+void quota_sync_sb(struct super_block *sb, int type);
 
 int dquot_initialize(struct inode *inode, int type);
 int dquot_drop(struct inode *inode);
@@ -253,12 +253,7 @@ static inline void vfs_dq_free_inode(str
 		inode->i_sb->dq_op->free_inode(inode, 1);
 }
 
-/* The following two functions cannot be called inside a transaction */
-static inline void vfs_dq_sync(struct super_block *sb)
-{
-	sync_dquots(sb, -1);
-}
-
+/* Cannot be called inside a transaction */
 static inline int vfs_dq_off(struct super_block *sb, int remount)
 {
 	int ret = -ENOSYS;
@@ -334,7 +329,7 @@ static inline void vfs_dq_free_inode(str
 {
 }
 
-static inline void vfs_dq_sync(struct super_block *sb)
+static inline void quota_sync_sb(struct super_block *sb, int type);
 {
 }
 

Re: [PATCH] quota: cleanup dquota sync functions

[Jan Kara]

  Hi,

On Sun 26-04-09 09:31:42, Christoph Hellwig wrote:
> Currently the VFS calls vfs_dq_sync to sync out disk quotas for a given
> superblock.  This is a small wrapper around sync_dquots which for the
> case of a non-NULL superblock is a small wrapper around quota_sync_sb.
> 
> Just make quota_sync_sb global and call it directly.  Also call it
> directly for those cases in quota.c that have a superblock and leave
> sync_dquots purely an iterator over quota_sync_sb and remove it's
> superblock argument.
> 
> To make this nicer move the check for the lack of a quota_sync method
> from the callers into quota_sync_sb.
> 
> Btw, I think calling quota_sync_sb for the !wait case in __fsync_super
> might be a bad idea because it calls into ->sync_fs with wait = 1.
> 
> Note that the patch requires Jan's sync rewrite applies first.
  Looks fine. Thanks. I've added it to the series of sync cleanups...

								Honza

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/fs/quota/quota.c
> ===================================================================
> --- linux-2.6.orig/fs/quota/quota.c	2009-04-25 19:53:49.074949184 +0200
> +++ linux-2.6/fs/quota/quota.c	2009-04-25 20:11:23.598952601 +0200
> @@ -159,10 +159,13 @@ static int check_quotactl_valid(struct s
>  	return error;
>  }
>  
> -static void quota_sync_sb(struct super_block *sb, int type)
> +void quota_sync_sb(struct super_block *sb, int type)
>  {
>  	int cnt;
>  
> +	if (!sb->s_qcop->quota_sync)
> +		return;
> +
>  	sb->s_qcop->quota_sync(sb, type);
>  
>  	if (sb_dqopt(sb)->flags & DQUOT_QUOTA_SYS_FILE)
> @@ -192,16 +195,11 @@ static void quota_sync_sb(struct super_b
>  	mutex_unlock(&sb_dqopt(sb)->dqonoff_mutex);
>  }
>  
> -void sync_dquots(struct super_block *sb, int type)
> +static void sync_dquots(int type)
>  {
> +	struct super_block *sb;
>  	int cnt;
>  
> -	if (sb) {
> -		if (sb->s_qcop->quota_sync)
> -			quota_sync_sb(sb, type);
> -		return;
> -	}
> -
>  	spin_lock(&sb_lock);
>  restart:
>  	list_for_each_entry(sb, &super_blocks, s_list) {
> @@ -222,7 +220,7 @@ restart:
>  		sb->s_count++;
>  		spin_unlock(&sb_lock);
>  		down_read(&sb->s_umount);
> -		if (sb->s_root && sb->s_qcop->quota_sync)
> +		if (sb->s_root)
>  			quota_sync_sb(sb, type);
>  		up_read(&sb->s_umount);
>  		spin_lock(&sb_lock);
> @@ -301,7 +299,10 @@ static int do_quotactl(struct super_bloc
>  			return sb->s_qcop->set_dqblk(sb, type, id, &idq);
>  		}
>  		case Q_SYNC:
> -			sync_dquots(sb, type);
> +			if (sb)
> +				quota_sync_sb(sb, type);
> +			else
> +				sync_dquots(type);
>  			return 0;
>  
>  		case Q_XQUOTAON:
> Index: linux-2.6/fs/super.c
> ===================================================================
> --- linux-2.6.orig/fs/super.c	2009-04-25 19:54:15.376074155 +0200
> +++ linux-2.6/fs/super.c	2009-04-25 19:54:16.125949480 +0200
> @@ -259,7 +259,7 @@ EXPORT_SYMBOL(unlock_super);
>  
>  static int __fsync_super(struct super_block *sb, int wait)
>  {
> -	vfs_dq_sync(sb);
> +	quota_sync_sb(sb, -1);
>  	sync_inodes_sb(sb, wait);
>  	lock_super(sb);
>  	if (sb->s_dirt && sb->s_op->write_super)
> Index: linux-2.6/include/linux/quotaops.h
> ===================================================================
> --- linux-2.6.orig/include/linux/quotaops.h	2009-04-25 19:53:49.086949524 +0200
> +++ linux-2.6/include/linux/quotaops.h	2009-04-25 19:54:16.126949327 +0200
> @@ -20,7 +20,7 @@ static inline struct quota_info *sb_dqop
>  /*
>   * declaration of quota_function calls in kernel.
>   */
> -void sync_dquots(struct super_block *sb, int type);
> +void quota_sync_sb(struct super_block *sb, int type);
>  
>  int dquot_initialize(struct inode *inode, int type);
>  int dquot_drop(struct inode *inode);
> @@ -253,12 +253,7 @@ static inline void vfs_dq_free_inode(str
>  		inode->i_sb->dq_op->free_inode(inode, 1);
>  }
>  
> -/* The following two functions cannot be called inside a transaction */
> -static inline void vfs_dq_sync(struct super_block *sb)
> -{
> -	sync_dquots(sb, -1);
> -}
> -
> +/* Cannot be called inside a transaction */
>  static inline int vfs_dq_off(struct super_block *sb, int remount)
>  {
>  	int ret = -ENOSYS;
> @@ -334,7 +329,7 @@ static inline void vfs_dq_free_inode(str
>  {
>  }
>  
> -static inline void vfs_dq_sync(struct super_block *sb)
> +static inline void quota_sync_sb(struct super_block *sb, int type);
>  {
>  }
>  
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] quota: cleanup dquota sync functions

[Jan Kara]

On Sun 26-04-09 09:31:42, Christoph Hellwig wrote:
> Btw, I think calling quota_sync_sb for the !wait case in __fsync_super
> might be a bad idea because it calls into ->sync_fs with wait = 1.
  Yes, I know this. I could hack it around so that quota is not synced at
all when wait == 0 but it would be a bit surprising to future callers. 
But we could just call ->quota_sync() when wait == 0. That would basically
do what's expected. I guess I'll do that.

									Honza

PS: I've changed quota_sync_sb() to sync_quota_sb() and fixed one compile
error when CONFIG_QUOTA is disabled in your patch.
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR