diff mbox

[7/7] quota: Remove dqonoff_mutex

Message ID 1479975162-24060-8-git-send-email-jack@suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kara Nov. 24, 2016, 8:12 a.m. UTC
The only places that were grabbing dqonoff_mutex are functions turning
quotas on and off and these are properly serialized using s_umount
semaphore. Remove dqonoff_mutex.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c      | 78 +++++++++++++++------------------------------------
 fs/super.c            |  1 -
 include/linux/quota.h |  1 -
 3 files changed, 23 insertions(+), 57 deletions(-)

Comments

kernel test robot Nov. 24, 2016, 12:55 p.m. UTC | #1
Hi Jan,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.9-rc6 next-20161124]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jan-Kara/fs-Provide-function-to-get-superblock-with-exclusive-s_umount/20161124-192840
config: x86_64-randconfig-s0-11241854 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   fs/quota/dquot.c: In function 'dquot_enable':
   fs/quota/dquot.c:2431: warning: label 'load_quota' defined but not used
>> fs/quota/dquot.c:2407: warning: unused variable 'dqopt'
   fs/quota/dquot.o: warning: objtool: inode_reserved_space()+0x20: function has unreachable instruction

vim +/dqopt +2407 fs/quota/dquot.c

f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2401   * of individual quota flags
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2402   */
287a8095 fs/quota/dquot.c Christoph Hellwig 2010-05-19  2403  int dquot_enable(struct inode *inode, int type, int format_id,
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2404  		 unsigned int flags)
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2405  {
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2406  	struct super_block *sb = inode->i_sb;
f55abc0f fs/dquot.c       Jan Kara          2008-08-20 @2407  	struct quota_info *dqopt = sb_dqopt(sb);
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2408  
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2409  	/* Just unsuspend quotas? */
0f0dd62f fs/quota/dquot.c Christoph Hellwig 2010-05-19  2410  	BUG_ON(flags & DQUOT_SUSPENDED);
a0a83c50 fs/quota/dquot.c Jan Kara          2016-11-24  2411  	/* s_umount should be held in exclusive mode */
a0a83c50 fs/quota/dquot.c Jan Kara          2016-11-24  2412  	if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount)))
a0a83c50 fs/quota/dquot.c Jan Kara          2016-11-24  2413  		up_read(&sb->s_umount);
0f0dd62f fs/quota/dquot.c Christoph Hellwig 2010-05-19  2414  
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2415  	if (!flags)
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2416  		return 0;
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2417  	/* Just updating flags needed? */
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2418  	if (sb_has_quota_loaded(sb, type)) {
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2419  		if (flags & DQUOT_USAGE_ENABLED &&
1d852305 fs/quota/dquot.c Jan Kara          2016-11-24  2420  		    sb_has_quota_usage_enabled(sb, type))
1d852305 fs/quota/dquot.c Jan Kara          2016-11-24  2421  			return -EBUSY;
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2422  		if (flags & DQUOT_LIMITS_ENABLED &&
1d852305 fs/quota/dquot.c Jan Kara          2016-11-24  2423  		    sb_has_quota_limits_enabled(sb, type))
1d852305 fs/quota/dquot.c Jan Kara          2016-11-24  2424  			return -EBUSY;
cc33412f fs/dquot.c       Jan Kara          2009-01-12  2425  		spin_lock(&dq_state_lock);
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2426  		sb_dqopt(sb)->flags |= dquot_state_flag(flags, type);
cc33412f fs/dquot.c       Jan Kara          2009-01-12  2427  		spin_unlock(&dq_state_lock);
1d852305 fs/quota/dquot.c Jan Kara          2016-11-24  2428  		return 0;
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2429  	}
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2430  
f55abc0f fs/dquot.c       Jan Kara          2008-08-20 @2431  load_quota:
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2432  	return vfs_load_quota_inode(inode, type, format_id, flags);
f55abc0f fs/dquot.c       Jan Kara          2008-08-20  2433  }
287a8095 fs/quota/dquot.c Christoph Hellwig 2010-05-19  2434  EXPORT_SYMBOL(dquot_enable);

:::::: The code at line 2407 was first introduced by commit
:::::: f55abc0fb9c3189de3da829adf3220322c0da43e quota: Allow to separately enable quota accounting and enforcing limits

:::::: TO: Jan Kara <jack@suse.cz>
:::::: CC: Mark Fasheh <mfasheh@suse.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index d91aecc939c9..0d9dccac39a8 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -119,8 +119,7 @@ 
  * spinlock to internal buffers before writing.
  *
  * Lock ordering (including related VFS locks) is the following:
- *   dqonoff_mutex > i_mutex > journal_lock > dquot->dq_lock > dqio_mutex
- * dqonoff_mutex > i_mutex comes from dquot_quota_sync, dquot_enable, etc.
+ *   s_umount > i_mutex > journal_lock > dquot->dq_lock > dqio_mutex
  */
 
 static __cacheline_aligned_in_smp DEFINE_SPINLOCK(dq_list_lock);
@@ -933,7 +932,7 @@  static int dqinit_needed(struct inode *inode, int type)
 	return 0;
 }
 
-/* This routine is guarded by dqonoff_mutex mutex */
+/* This routine is guarded by s_umount semaphore */
 static void add_dquot_ref(struct super_block *sb, int type)
 {
 	struct inode *inode, *old_inode = NULL;
@@ -2108,18 +2107,14 @@  int dquot_disable(struct super_block *sb, int type, unsigned int flags)
 	    DQUOT_USAGE_ENABLED)))
 		return -EINVAL;
 
-	/* We need to serialize quota_off() for device */
-	mutex_lock(&dqopt->dqonoff_mutex);
-
 	/*
 	 * Skip everything if there's nothing to do. We have to do this because
 	 * sometimes we are called when fill_super() failed and calling
 	 * sync_fs() in such cases does no good.
 	 */
-	if (!sb_any_quota_loaded(sb)) {
-		mutex_unlock(&dqopt->dqonoff_mutex);
+	if (!sb_any_quota_loaded(sb))
 		return 0;
-	}
+
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		toputinode[cnt] = NULL;
 		if (type != -1 && cnt != type)
@@ -2173,7 +2168,6 @@  int dquot_disable(struct super_block *sb, int type, unsigned int flags)
 		dqopt->info[cnt].dqi_bgrace = 0;
 		dqopt->ops[cnt] = NULL;
 	}
-	mutex_unlock(&dqopt->dqonoff_mutex);
 
 	/* Skip syncing and setting flags if quota files are hidden */
 	if (dqopt->flags & DQUOT_QUOTA_SYS_FILE)
@@ -2191,19 +2185,13 @@  int dquot_disable(struct super_block *sb, int type, unsigned int flags)
 	 * changes done by userspace on the next quotaon() */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
 		if (toputinode[cnt]) {
-			mutex_lock(&dqopt->dqonoff_mutex);
-			/* If quota was reenabled in the meantime, we have
-			 * nothing to do */
-			if (!sb_has_quota_loaded(sb, cnt)) {
-				inode_lock(toputinode[cnt]);
-				toputinode[cnt]->i_flags &= ~(S_IMMUTABLE |
+			WARN_ON_ONCE(sb_has_quota_loaded(sb, cnt));
+			inode_lock(toputinode[cnt]);
+			toputinode[cnt]->i_flags &= ~(S_IMMUTABLE |
 				  S_NOATIME | S_NOQUOTA);
-				truncate_inode_pages(&toputinode[cnt]->i_data,
-						     0);
-				inode_unlock(toputinode[cnt]);
-				mark_inode_dirty_sync(toputinode[cnt]);
-			}
-			mutex_unlock(&dqopt->dqonoff_mutex);
+			truncate_inode_pages(&toputinode[cnt]->i_data, 0);
+			inode_unlock(toputinode[cnt]);
+			mark_inode_dirty_sync(toputinode[cnt]);
 		}
 	if (sb->s_bdev)
 		invalidate_bdev(sb->s_bdev);
@@ -2275,6 +2263,10 @@  static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 		error = -EINVAL;
 		goto out_fmt;
 	}
+	if (sb_has_quota_loaded(sb, type)) {
+		error = -EBUSY;
+		goto out_fmt;
+	}
 
 	if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) {
 		/* As we bypass the pagecache we must now flush all the
@@ -2286,11 +2278,6 @@  static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 		sync_filesystem(sb);
 		invalidate_bdev(sb->s_bdev);
 	}
-	mutex_lock(&dqopt->dqonoff_mutex);
-	if (sb_has_quota_loaded(sb, type)) {
-		error = -EBUSY;
-		goto out_lock;
-	}
 
 	if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) {
 		/* We don't want quota and atime on quota files (deadlocks
@@ -2311,7 +2298,7 @@  static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 	error = -EIO;
 	dqopt->files[type] = igrab(inode);
 	if (!dqopt->files[type])
-		goto out_lock;
+		goto out_file_flags;
 	error = -EINVAL;
 	if (!fmt->qf_ops->check_quota_file(sb, type))
 		goto out_file_init;
@@ -2334,14 +2321,13 @@  static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 	spin_unlock(&dq_state_lock);
 
 	add_dquot_ref(sb, type);
-	mutex_unlock(&dqopt->dqonoff_mutex);
 
 	return 0;
 
 out_file_init:
 	dqopt->files[type] = NULL;
 	iput(inode);
-out_lock:
+out_file_flags:
 	if (oldflags != -1) {
 		inode_lock(inode);
 		/* Set the flags back (in the case of accidental quotaon()
@@ -2350,7 +2336,6 @@  static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 		inode->i_flags |= oldflags;
 		inode_unlock(inode);
 	}
-	mutex_unlock(&dqopt->dqonoff_mutex);
 out_fmt:
 	put_quota_format(fmt);
 
@@ -2372,12 +2357,9 @@  int dquot_resume(struct super_block *sb, int type)
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		if (type != -1 && cnt != type)
 			continue;
-
-		mutex_lock(&dqopt->dqonoff_mutex);
-		if (!sb_has_quota_suspended(sb, cnt)) {
-			mutex_unlock(&dqopt->dqonoff_mutex);
+		if (!sb_has_quota_suspended(sb, cnt))
 			continue;
-		}
+
 		inode = dqopt->files[cnt];
 		dqopt->files[cnt] = NULL;
 		spin_lock(&dq_state_lock);
@@ -2386,7 +2368,6 @@  int dquot_resume(struct super_block *sb, int type)
 							cnt);
 		dqopt->flags &= ~dquot_state_flag(DQUOT_STATE_FLAGS, cnt);
 		spin_unlock(&dq_state_lock);
-		mutex_unlock(&dqopt->dqonoff_mutex);
 
 		flags = dquot_generic_flag(flags, cnt);
 		ret = vfs_load_quota_inode(inode, cnt,
@@ -2422,7 +2403,6 @@  EXPORT_SYMBOL(dquot_quota_on);
 int dquot_enable(struct inode *inode, int type, int format_id,
 		 unsigned int flags)
 {
-	int ret = 0;
 	struct super_block *sb = inode->i_sb;
 	struct quota_info *dqopt = sb_dqopt(sb);
 
@@ -2436,28 +2416,16 @@  int dquot_enable(struct inode *inode, int type, int format_id,
 		return 0;
 	/* Just updating flags needed? */
 	if (sb_has_quota_loaded(sb, type)) {
-		mutex_lock(&dqopt->dqonoff_mutex);
-		/* Now do a reliable test... */
-		if (!sb_has_quota_loaded(sb, type)) {
-			mutex_unlock(&dqopt->dqonoff_mutex);
-			goto load_quota;
-		}
 		if (flags & DQUOT_USAGE_ENABLED &&
-		    sb_has_quota_usage_enabled(sb, type)) {
-			ret = -EBUSY;
-			goto out_lock;
-		}
+		    sb_has_quota_usage_enabled(sb, type))
+			return -EBUSY;
 		if (flags & DQUOT_LIMITS_ENABLED &&
-		    sb_has_quota_limits_enabled(sb, type)) {
-			ret = -EBUSY;
-			goto out_lock;
-		}
+		    sb_has_quota_limits_enabled(sb, type))
+			return -EBUSY;
 		spin_lock(&dq_state_lock);
 		sb_dqopt(sb)->flags |= dquot_state_flag(flags, type);
 		spin_unlock(&dq_state_lock);
-out_lock:
-		mutex_unlock(&dqopt->dqonoff_mutex);
-		return ret;
+		return 0;
 	}
 
 load_quota:
diff --git a/fs/super.c b/fs/super.c
index f7f724230e2b..1709ed029a2c 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -244,7 +244,6 @@  static struct super_block *alloc_super(struct file_system_type *type, int flags,
 	mutex_init(&s->s_vfs_rename_mutex);
 	lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
 	mutex_init(&s->s_dquot.dqio_mutex);
-	mutex_init(&s->s_dquot.dqonoff_mutex);
 	s->s_maxbytes = MAX_NON_LFS;
 	s->s_op = &default_op;
 	s->s_time_gran = 1000000000;
diff --git a/include/linux/quota.h b/include/linux/quota.h
index 55107a8ff887..b281d198ee5b 100644
--- a/include/linux/quota.h
+++ b/include/linux/quota.h
@@ -520,7 +520,6 @@  static inline void quota_send_warning(struct kqid qid, dev_t dev,
 struct quota_info {
 	unsigned int flags;			/* Flags for diskquotas on this device */
 	struct mutex dqio_mutex;		/* lock device while I/O in progress */
-	struct mutex dqonoff_mutex;		/* Serialize quotaon & quotaoff */
 	struct inode *files[MAXQUOTAS];		/* inodes of quotafiles */
 	struct mem_dqinfo info[MAXQUOTAS];	/* Information for each quota type */
 	const struct quota_format_ops *ops[MAXQUOTAS];	/* Operations for each type */