[2/7] quota: Hold s_umount in exclusive mode when enabling / disabling quotas

Commit Message

Jan Kara Nov. 24, 2016, 8:12 a.m. UTC

Currently we hold s_umount semaphore only in shared mode when enabling
or disabling quotas and use dqonoff_mutex for serializing quota state
changes on a filesystem and also quota state changes with other places
depending on current quota state. Using dedicated mutex for this causes
possible deadlocks during filesystem freezing (see following commit for
details) so we transition to using s_umount semaphore for the necessary
synchronization whose lock ordering is properly handled by the
filesystem freezing code. As a start grab s_umount in exclusive mode
when enabling / disabling quotas.

Signed-off-by: Jan Kara <[e-mail]>
---
 fs/quota/dquot.c | 11 +++++++++++
 fs/quota/quota.c | 15 +++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

Comments

kernel test robot Nov. 24, 2016, 11:49 a.m. UTC | #1

Hi Jan,

[auto build test ERROR on linus/master]
[also build test ERROR 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-x013-201647 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/quota/quota.c: In function 'SYSC_quotactl':
>> fs/quota/quota.c:883:7: error: implicit declaration of function 'quotactl_cmd_onoff' [-Werror=implicit-function-declaration]
     if (!quotactl_cmd_onoff(cmds))
          ^~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/quotactl_cmd_onoff +883 fs/quota/quota.c

   877			ret = PTR_ERR(sb);
   878			goto out;
   879		}
   880	
   881		ret = do_quotactl(sb, type, cmds, id, addr, pathp);
   882	
 > 883		if (!quotactl_cmd_onoff(cmds))
   884			drop_super(sb);
   885		else
   886			drop_super_exclusive(sb);

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

Patch

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 1bfac28b7e7d..047afb966420 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2107,6 +2107,10 @@  int dquot_disable(struct super_block *sb, int type, unsigned int flags)
 	struct quota_info *dqopt = sb_dqopt(sb);
 	struct inode *toputinode[MAXQUOTAS];
 
+	/* s_umount should be held in exclusive mode */
+	if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount)))
+		up_read(&sb->s_umount);
+
 	/* Cannot turn off usage accounting without turning off limits, or
 	 * suspend quotas and simultaneously turn quotas off. */
 	if ((flags & DQUOT_USAGE_ENABLED && !(flags & DQUOT_LIMITS_ENABLED))
@@ -2371,6 +2375,10 @@  int dquot_resume(struct super_block *sb, int type)
 	int ret = 0, cnt;
 	unsigned int flags;
 
+	/* s_umount should be held in exclusive mode */
+	if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount)))
+		up_read(&sb->s_umount);
+
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		if (type != -1 && cnt != type)
 			continue;
@@ -2430,6 +2438,9 @@  int dquot_enable(struct inode *inode, int type, int format_id,
 
 	/* Just unsuspend quotas? */
 	BUG_ON(flags & DQUOT_SUSPENDED);
+	/* s_umount should be held in exclusive mode */
+	if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount)))
+		up_read(&sb->s_umount);
 
 	if (!flags)
 		return 0;
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 2d445425aad7..790c61abc663 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -790,6 +790,12 @@  static int quotactl_cmd_write(int cmd)
 	return 1;
 }
 
+/* Return true if quotactl command is manipulating quota on/off state */
+static bool quotactl_cmd_onoff(int cmd)
+{
+	return (cmd == Q_QUOTAON) || (cmd == Q_QUOTAOFF);
+}
+
 #endif /* CONFIG_BLOCK */
 
 /*
@@ -809,7 +815,9 @@  static struct super_block *quotactl_block(const char __user *special, int cmd)
 	putname(tmp);
 	if (IS_ERR(bdev))
 		return ERR_CAST(bdev);
-	if (quotactl_cmd_write(cmd))
+	if (quotactl_cmd_onoff(cmd))
+		sb = get_super_exclusive_thawed(bdev);
+	else if (quotactl_cmd_write(cmd))
 		sb = get_super_thawed(bdev);
 	else
 		sb = get_super(bdev);
@@ -872,7 +880,10 @@  SYSCALL_DEFINE4(quotactl, unsigned int, cmd, const char __user *, special,
 
 	ret = do_quotactl(sb, type, cmds, id, addr, pathp);
 
-	drop_super(sb);
+	if (!quotactl_cmd_onoff(cmds))
+		drop_super(sb);
+	else
+		drop_super_exclusive(sb);
 out:
 	if (pathp && !IS_ERR(pathp))
 		path_put(pathp);