Skip to contentSkip to conversation

: Remove redundant underlying file system checking that the quota files resides from dquot_quota_on()

PATCH In review Jeff Liu 2 participants 3 messages Apr 22, 2012 – Apr 23, 2012

Subsystems (2)
lore
Share & tools
lore

Jeff Liu

author
#0 lore raw

PATCH 1/1 : Remove redundant underlying file system checking that the quota files resides from dquot_quota_on()

Hi Jan,

When executing quotaon(8), all the file systems(ext3/ext4/reiserfs) binded to the VFS quota framework have already ensured that the quota files are resides on the same file system with the specified super block before invoking dquot_quota_on(), looks like we can be safely remove it from there IMHO.

---
 fs/quota/dquot.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)
fs/quota/dquot.c +4 -7
index 4674197..cb917a2 100644
@@ -2199,13 +2199,10 @@ int dquot_quota_on(struct super_block *sb, int type, int format_id,
 	int error = security_quota_on(path->dentry);
 	if (error)
 		return error;
-	/* Quota file not on the same filesystem? */
-	if (path->dentry->d_sb != sb)
-		error = -EXDEV;
-	else
-		error = vfs_load_quota_inode(path->dentry->d_inode, type,
-					     format_id, DQUOT_USAGE_ENABLED |
-					     DQUOT_LIMITS_ENABLED);
+
+	error = vfs_load_quota_inode(path->dentry->d_inode, type,
+				     format_id, DQUOT_USAGE_ENABLED |
+				     DQUOT_LIMITS_ENABLED);
 	return error;
 }
 EXPORT_SYMBOL(dquot_quota_on);
Signature · 1 line
1.7.9
reviewer
#1 lore raw

Hi,

Quoted 5 lines · Jeff Liu

When executing quotaon(8), all the file systems(ext3/ext4/reiserfs) binded to the VFS quota framework have already ensured that the quota files are resides on the same file system with the specified super block before invoking dquot_quota_on(), looks like we can be safely remove it from there IMHO.

Not really, ext2 doesn't perform the check. Also ext3 or reiserfs when
not using journalled quota don't do the check. So the test has to stay there. Is there any special reason why you would like to have it removed?

Honza

Quoted 28 lines
---
 fs/quota/dquot.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)
fs/quota/dquot.c +4 -7
index 4674197..cb917a2 100644
@@ -2199,13 +2199,10 @@ int dquot_quota_on(struct super_block *sb, int type, int format_id,
 	int error = security_quota_on(path->dentry);
 	if (error)
 		return error;
-	/* Quota file not on the same filesystem? */
-	if (path->dentry->d_sb != sb)
-		error = -EXDEV;
-	else
-		error = vfs_load_quota_inode(path->dentry->d_inode, type,
-					     format_id, DQUOT_USAGE_ENABLED |
-					     DQUOT_LIMITS_ENABLED);
+
+	error = vfs_load_quota_inode(path->dentry->d_inode, type,
+				     format_id, DQUOT_USAGE_ENABLED |
+				     DQUOT_LIMITS_ENABLED);
 	return error;
 }
 EXPORT_SYMBOL(dquot_quota_on);
Signature · 1 line
1.7.9
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
author
#2 lore raw

Hi Jan,

Quoted 11 lines · Jan Kara

Hi,

Quoted 5 lines · Jeff Liu

When executing quotaon(8), all the file systems(ext3/ext4/reiserfs) binded to the VFS quota framework have already ensured that the quota files are resides on the same file system with the specified super block before invoking dquot_quota_on(), looks like we can be safely remove it from there IMHO.

Not really, ext2 doesn't perform the check. Also ext3 or reiserfs when
not using journalled quota don't do the check. So the test has to stay there. Is there any special reason why you would like to have it removed?

No special reason, I just found that this check has been done at the beginning of ext3/ext4/reiserfs->(ext3/ext4/reiserfs)_quota_on() for journalled quota, but not realized that ext2 quota works without journall. :(
Sorry for the noise!

Thanks,
-Jeff

Quoted 31 lines

Honza

Quoted 28 lines
---
 fs/quota/dquot.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)
fs/quota/dquot.c +4 -7
index 4674197..cb917a2 100644
@@ -2199,13 +2199,10 @@ int dquot_quota_on(struct super_block *sb, int type, int format_id,
 	int error = security_quota_on(path->dentry);
 	if (error)
 		return error;
-	/* Quota file not on the same filesystem? */
-	if (path->dentry->d_sb != sb)
-		error = -EXDEV;
-	else
-		error = vfs_load_quota_inode(path->dentry->d_inode, type,
-					     format_id, DQUOT_USAGE_ENABLED |
-					     DQUOT_LIMITS_ENABLED);
+
+	error = vfs_load_quota_inode(path->dentry->d_inode, type,
+				     format_id, DQUOT_USAGE_ENABLED |
+				     DQUOT_LIMITS_ENABLED);
 	return error;
 }
 EXPORT_SYMBOL(dquot_quota_on);
Signature · 1 line
1.7.9