[PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly

[PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly

[Jan Kara]

Hello,

this series removes setting of IMMUTABLE and NOATIME flags from generic quota
code and moves it to filesystem's quota_on / quota_off handlers. Since this
was the only place where generic code was modifying inode flags that need to
be persistent on disk, we can remove functions syncing inode->i_flags to
on-disk flags after this. This was motivated by recent statx() work that
wants consistent view of inode flags without holding i_mutex.

Patches passed basic testing for ext2, ext4, reiserfs, compile-tested for jfs
only. Please review, patches are mostly trivial but I could have missed
something. If people are fine with the changes, I plan to merge them through
my tree.

								Honza

[PATCH 01/11] ext4: Set flags on quota files directly

[Jan Kara]

Currently immutable and noatime flags on quota files are set by quota
code which requires us to copy inode->i_flags to our on disk version of
quota flags in GETFLAGS ioctl and ext4_do_update_inode(). Move to
setting / clearing these on-disk flags directly to save that copying.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a9448db1cf7e..480cbbebdc57 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -839,6 +839,28 @@ static void dump_orphan_list(struct super_block *sb, struct ext4_sb_info *sbi)
 	}
 }
 
+#ifdef CONFIG_QUOTA
+static int ext4_quota_off(struct super_block *sb, int type);
+
+static inline void ext4_quota_off_umount(struct super_block *sb)
+{
+	int type;
+
+	if (ext4_has_feature_quota(sb)) {
+		dquot_disable(sb, -1,
+			      DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
+	} else {
+		/* Use our quota_off function to clear inode flags etc. */
+		for (type = 0; type < EXT4_MAXQUOTAS; type++)
+			ext4_quota_off(sb, type);
+	}
+}
+#else
+static inline void ext4_quota_off_umount(struct super_block *sb)
+{
+}
+#endif
+
 static void ext4_put_super(struct super_block *sb)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -847,7 +869,7 @@ static void ext4_put_super(struct super_block *sb)
 	int i, err;
 
 	ext4_unregister_li_request(sb);
-	dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
+	ext4_quota_off_umount(sb);
 
 	flush_workqueue(sbi->rsv_conversion_wq);
 	destroy_workqueue(sbi->rsv_conversion_wq);
@@ -1218,7 +1240,6 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot);
 static int ext4_write_info(struct super_block *sb, int type);
 static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 			 const struct path *path);
-static int ext4_quota_off(struct super_block *sb, int type);
 static int ext4_quota_on_mount(struct super_block *sb, int type);
 static ssize_t ext4_quota_read(struct super_block *sb, int type, char *data,
 			       size_t len, loff_t off);
@@ -5344,11 +5365,28 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
 		if (err)
 			return err;
 	}
+
 	lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
 	err = dquot_quota_on(sb, type, format_id, path);
-	if (err)
+	if (err) {
 		lockdep_set_quota_inode(path->dentry->d_inode,
 					     I_DATA_SEM_NORMAL);
+	} else {
+		struct inode *inode = d_inode(path->dentry);
+		handle_t *handle;
+
+		inode_lock(inode);
+		handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
+		if (IS_ERR(handle))
+			goto unlock_inode;
+		EXT4_I(inode)->i_flags |= EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL;
+		inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
+				S_NOATIME | S_IMMUTABLE);
+		ext4_mark_inode_dirty(handle, inode);
+		ext4_journal_stop(handle);
+	unlock_inode:
+		inode_unlock(inode);
+	}
 	return err;
 }
 
@@ -5422,24 +5460,36 @@ static int ext4_quota_off(struct super_block *sb, int type)
 {
 	struct inode *inode = sb_dqopt(sb)->files[type];
 	handle_t *handle;
+	int err;
 
 	/* Force all delayed allocation blocks to be allocated.
 	 * Caller already holds s_umount sem */
 	if (test_opt(sb, DELALLOC))
 		sync_filesystem(sb);
 
-	if (!inode)
+	if (!inode || !igrab(inode))
 		goto out;
 
+	err = dquot_quota_off(sb, type);
+	if (err)
+		goto out_put;
+
+	inode_lock(inode);
 	/* Update modification times of quota files when userspace can
 	 * start looking at them */
 	handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
 	if (IS_ERR(handle))
-		goto out;
+		goto out_unlock;
+	EXT4_I(inode)->i_flags &= ~(EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL);
+	inode_set_flags(inode, 0, S_NOATIME | S_IMMUTABLE);
 	inode->i_mtime = inode->i_ctime = current_time(inode);
 	ext4_mark_inode_dirty(handle, inode);
 	ext4_journal_stop(handle);
-
+out_unlock:
+	inode_unlock(inode);
+out_put:
+	iput(inode);
+	return err;
 out:
 	return dquot_quota_off(sb, type);
 }
-- 
2.12.0

Re: [PATCH 01/11] ext4: Set flags on quota files directly

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 4856 bytes --]

On Apr 12, 2017, at 1:26 AM, Jan Kara <jack@suse.cz> wrote:
> 
> Currently immutable and noatime flags on quota files are set by quota
> code which requires us to copy inode->i_flags to our on disk version of
> quota flags in GETFLAGS ioctl and ext4_do_update_inode(). Move to
> setting / clearing these on-disk flags directly to save that copying.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/ext4/super.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index a9448db1cf7e..480cbbebdc57 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -839,6 +839,28 @@ static void dump_orphan_list(struct super_block *sb, struct ext4_sb_info *sbi)
> 	}
> }
> 
> +#ifdef CONFIG_QUOTA
> +static int ext4_quota_off(struct super_block *sb, int type);
> +
> +static inline void ext4_quota_off_umount(struct super_block *sb)
> +{
> +	int type;
> +
> +	if (ext4_has_feature_quota(sb)) {
> +		dquot_disable(sb, -1,
> +			      DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
> +	} else {
> +		/* Use our quota_off function to clear inode flags etc. */
> +		for (type = 0; type < EXT4_MAXQUOTAS; type++)
> +			ext4_quota_off(sb, type);
> +	}
> +}
> +#else
> +static inline void ext4_quota_off_umount(struct super_block *sb)
> +{
> +}
> +#endif
> +
> static void ext4_put_super(struct super_block *sb)
> {
> 	struct ext4_sb_info *sbi = EXT4_SB(sb);
> @@ -847,7 +869,7 @@ static void ext4_put_super(struct super_block *sb)
> 	int i, err;
> 
> 	ext4_unregister_li_request(sb);
> -	dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
> +	ext4_quota_off_umount(sb);
> 
> 	flush_workqueue(sbi->rsv_conversion_wq);
> 	destroy_workqueue(sbi->rsv_conversion_wq);
> @@ -1218,7 +1240,6 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot);
> static int ext4_write_info(struct super_block *sb, int type);
> static int ext4_quota_on(struct super_block *sb, int type, int format_id,
> 			 const struct path *path);
> -static int ext4_quota_off(struct super_block *sb, int type);
> static int ext4_quota_on_mount(struct super_block *sb, int type);
> static ssize_t ext4_quota_read(struct super_block *sb, int type, char *data,
> 			       size_t len, loff_t off);
> @@ -5344,11 +5365,28 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
> 		if (err)
> 			return err;
> 	}
> +
> 	lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
> 	err = dquot_quota_on(sb, type, format_id, path);
> -	if (err)
> +	if (err) {
> 		lockdep_set_quota_inode(path->dentry->d_inode,
> 					     I_DATA_SEM_NORMAL);
> +	} else {
> +		struct inode *inode = d_inode(path->dentry);
> +		handle_t *handle;
> +
> +		inode_lock(inode);
> +		handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> +		if (IS_ERR(handle))
> +			goto unlock_inode;
> +		EXT4_I(inode)->i_flags |= EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL;
> +		inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
> +				S_NOATIME | S_IMMUTABLE);
> +		ext4_mark_inode_dirty(handle, inode);
> +		ext4_journal_stop(handle);

Should this be conditional on these flags not already being set?  Otherwise
it dirties the filesystem on every mount even if it isn't needed.

> +	unlock_inode:
> +		inode_unlock(inode);
> +	}
> 	return err;
> }
> 
> @@ -5422,24 +5460,36 @@ static int ext4_quota_off(struct super_block *sb, int type)
> {
> 	struct inode *inode = sb_dqopt(sb)->files[type];
> 	handle_t *handle;
> +	int err;
> 
> 	/* Force all delayed allocation blocks to be allocated.
> 	 * Caller already holds s_umount sem */
> 	if (test_opt(sb, DELALLOC))
> 		sync_filesystem(sb);
> 
> -	if (!inode)
> +	if (!inode || !igrab(inode))
> 		goto out;
> 
> +	err = dquot_quota_off(sb, type);
> +	if (err)
> +		goto out_put;
> +
> +	inode_lock(inode);
> 	/* Update modification times of quota files when userspace can
> 	 * start looking at them */
> 	handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> 	if (IS_ERR(handle))
> -		goto out;
> +		goto out_unlock;
> +	EXT4_I(inode)->i_flags &= ~(EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL);
> +	inode_set_flags(inode, 0, S_NOATIME | S_IMMUTABLE);

It isn't obvious that we need to clear these flags at every unmount?  I
don't think there is a good reason to be modifying the quota files from
userspace, and e2fsck won't care about these flags anyway when fixing
the quota files.

> 	inode->i_mtime = inode->i_ctime = current_time(inode);
> 	ext4_mark_inode_dirty(handle, inode);
> 	ext4_journal_stop(handle);
> -
> +out_unlock:
> +	inode_unlock(inode);
> +out_put:
> +	iput(inode);
> +	return err;
> out:
> 	return dquot_quota_off(sb, type);
> }
> --
> 2.12.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 01/11] ext4: Set flags on quota files directly

[Jan Kara]

On Wed 12-04-17 15:00:04, Andreas Dilger wrote:
> On Apr 12, 2017, at 1:26 AM, Jan Kara <jack@suse.cz> wrote:
> > @@ -5344,11 +5365,28 @@ static int ext4_quota_on(struct super_block *sb, int type, int format_id,
> > 		if (err)
> > 			return err;
> > 	}
> > +
> > 	lockdep_set_quota_inode(path->dentry->d_inode, I_DATA_SEM_QUOTA);
> > 	err = dquot_quota_on(sb, type, format_id, path);
> > -	if (err)
> > +	if (err) {
> > 		lockdep_set_quota_inode(path->dentry->d_inode,
> > 					     I_DATA_SEM_NORMAL);
> > +	} else {
> > +		struct inode *inode = d_inode(path->dentry);
> > +		handle_t *handle;
> > +
> > +		inode_lock(inode);
> > +		handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> > +		if (IS_ERR(handle))
> > +			goto unlock_inode;
> > +		EXT4_I(inode)->i_flags |= EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL;
> > +		inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
> > +				S_NOATIME | S_IMMUTABLE);
> > +		ext4_mark_inode_dirty(handle, inode);
> > +		ext4_journal_stop(handle);
> 
> Should this be conditional on these flags not already being set?  Otherwise
> it dirties the filesystem on every mount even if it isn't needed.

Well, this doesn't happen on mount but on Q_QUOTAON quotactl and only when
INCOMPAT_QUOTA feature is not enabled (i.e., we are using legacy quota
support). Also the flags are not expected to be set as userspace
quota-tools (most notably quotacheck) need to write to quota files when
kernel isn't using them.

> > @@ -5422,24 +5460,36 @@ static int ext4_quota_off(struct super_block *sb, int type)
> > {
> > 	struct inode *inode = sb_dqopt(sb)->files[type];
> > 	handle_t *handle;
> > +	int err;
> > 
> > 	/* Force all delayed allocation blocks to be allocated.
> > 	 * Caller already holds s_umount sem */
> > 	if (test_opt(sb, DELALLOC))
> > 		sync_filesystem(sb);
> > 
> > -	if (!inode)
> > +	if (!inode || !igrab(inode))
> > 		goto out;
> > 
> > +	err = dquot_quota_off(sb, type);
> > +	if (err)
> > +		goto out_put;
> > +
> > +	inode_lock(inode);
> > 	/* Update modification times of quota files when userspace can
> > 	 * start looking at them */
> > 	handle = ext4_journal_start(inode, EXT4_HT_QUOTA, 1);
> > 	if (IS_ERR(handle))
> > -		goto out;
> > +		goto out_unlock;
> > +	EXT4_I(inode)->i_flags &= ~(EXT4_NOATIME_FL | EXT4_IMMUTABLE_FL);
> > +	inode_set_flags(inode, 0, S_NOATIME | S_IMMUTABLE);
> 
> It isn't obvious that we need to clear these flags at every unmount?  I
> don't think there is a good reason to be modifying the quota files from
> userspace, and e2fsck won't care about these flags anyway when fixing
> the quota files.

Remember we are in the case of legacy quota support. In that case e2fsck
doesn't touch quota files and quotacheck is used instead to put quota files
in sync with what is in the filesystem. Also tools like setquota directly
modify quota files when kernel isn't using them. So we must clear the flags
on quotaoff to keep all this working.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 02/11] reiserfs: Set flags on quota files directly

[Jan Kara]

Currently immutable and noatime flags on quota files are set by quota
code which requires us to copy inode->i_flags to our on disk version of
quota flags in GETFLAGS ioctl and when writing stat item. Move to
setting / clearing these on-disk flags directly to save that copying.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/reiserfs/super.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index feabcde0290d..1fbb687fb83e 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -547,12 +547,28 @@ static void reiserfs_kill_sb(struct super_block *s)
 	kill_block_super(s);
 }
 
+#ifdef CONFIG_QUOTA
+static int reiserfs_quota_off(struct super_block *sb, int type);
+
+static void reiserfs_quota_off_umount(struct super_block *s)
+{
+	int type;
+
+	for (type = 0; type < REISERFS_MAXQUOTAS; type++)
+		reiserfs_quota_off(s, type);
+}
+#else
+static inline void reiserfs_quota_off_umount(struct super_block *s)
+{
+}
+#endif
+
 static void reiserfs_put_super(struct super_block *s)
 {
 	struct reiserfs_transaction_handle th;
 	th.t_trans_id = 0;
 
-	dquot_disable(s, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
+	reiserfs_quota_off_umount(s);
 
 	reiserfs_write_lock(s);
 
@@ -817,7 +833,7 @@ static const struct dquot_operations reiserfs_quota_operations = {
 
 static const struct quotactl_ops reiserfs_qctl_operations = {
 	.quota_on = reiserfs_quota_on,
-	.quota_off = dquot_quota_off,
+	.quota_off = reiserfs_quota_off,
 	.quota_sync = dquot_quota_sync,
 	.get_state = dquot_get_state,
 	.set_info = dquot_set_dqinfo,
@@ -2405,12 +2421,47 @@ static int reiserfs_quota_on(struct super_block *sb, int type, int format_id,
 			goto out;
 	}
 	reiserfs_write_unlock(sb);
-	return dquot_quota_on(sb, type, format_id, path);
+	err = dquot_quota_on(sb, type, format_id, path);
+	if (!err) {
+		inode_lock(inode);
+		REISERFS_I(inode)->i_attrs |= REISERFS_IMMUTABLE_FL |
+					      REISERFS_NOATIME_FL;
+		inode_set_flags(inode, S_IMMUTABLE | S_NOATIME,
+				S_IMMUTABLE | S_NOATIME);
+		inode_unlock(inode);
+		mark_inode_dirty(inode);
+	}
+	return err;
 out:
 	reiserfs_write_unlock(sb);
 	return err;
 }
 
+static int reiserfs_quota_off(struct super_block *sb, int type)
+{
+	int err;
+	struct inode *inode = sb_dqopt(sb)->files[type];
+
+	if (!inode || !igrab(inode))
+		goto out;
+
+	err = dquot_quota_off(sb, type);
+	if (err)
+		goto out_put;
+
+	inode_lock(inode);
+	REISERFS_I(inode)->i_attrs &= ~(REISERFS_IMMUTABLE_FL |
+					REISERFS_NOATIME_FL);
+	inode_set_flags(inode, 0, S_IMMUTABLE | S_NOATIME);
+	inode_unlock(inode);
+	mark_inode_dirty(inode);
+out_put:
+	iput(inode);
+	return err;
+out:
+	return dquot_quota_off(sb, type);
+}
+
 /*
  * Read data from quotafile - avoid pagecache and such because we cannot afford
  * acquiring the locks... As quota files are never truncated and quota code
-- 
2.12.0

[PATCH 03/11] ext2: Set flags on quota files directly

[Jan Kara]

Currently immutable and noatime flags on quota files are set by quota
code which requires us to copy inode->i_flags to our on disk version of
quota flags in GETFLAGS ioctl and __ext2_write_inode().  Move to setting
/ clearing these on-disk flags directly to save that copying.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/super.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 9e25a71fe1a2..ca9ad24441cb 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -123,13 +123,29 @@ void ext2_update_dynamic_rev(struct super_block *sb)
 	 */
 }
 
+#ifdef CONFIG_QUOTA
+static int ext2_quota_off(struct super_block *sb, int type);
+
+static void ext2_quota_off_umount(struct super_block *sb)
+{
+	int type;
+
+	for (type = 0; type < MAXQUOTAS; type++)
+		ext2_quota_off(sb, type);
+}
+#else
+static inline void ext2_quota_off_umount(struct super_block *sb)
+{
+}
+#endif
+
 static void ext2_put_super (struct super_block * sb)
 {
 	int db_count;
 	int i;
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 
-	dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
+	ext2_quota_off_umount(sb);
 
 	if (sbi->s_mb_cache) {
 		ext2_xattr_destroy_cache(sbi->s_mb_cache);
@@ -314,10 +330,23 @@ static int ext2_show_options(struct seq_file *seq, struct dentry *root)
 #ifdef CONFIG_QUOTA
 static ssize_t ext2_quota_read(struct super_block *sb, int type, char *data, size_t len, loff_t off);
 static ssize_t ext2_quota_write(struct super_block *sb, int type, const char *data, size_t len, loff_t off);
+static int ext2_quota_on(struct super_block *sb, int type, int format_id,
+			 const struct path *path);
 static struct dquot **ext2_get_dquots(struct inode *inode)
 {
 	return EXT2_I(inode)->i_dquot;
 }
+
+static const struct quotactl_ops ext2_quotactl_ops = {
+	.quota_on	= ext2_quota_on,
+	.quota_off	= ext2_quota_off,
+	.quota_sync	= dquot_quota_sync,
+	.get_state	= dquot_get_state,
+	.set_info	= dquot_set_dqinfo,
+	.get_dqblk	= dquot_get_dqblk,
+	.set_dqblk	= dquot_set_dqblk,
+	.get_nextdqblk	= dquot_get_next_dqblk,
+};
 #endif
 
 static const struct super_operations ext2_sops = {
@@ -1117,7 +1146,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
-	sb->s_qcop = &dquot_quotactl_ops;
+	sb->s_qcop = &ext2_quotactl_ops;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 #endif
 
@@ -1548,6 +1577,51 @@ static ssize_t ext2_quota_write(struct super_block *sb, int type,
 	return len - towrite;
 }
 
+static int ext2_quota_on(struct super_block *sb, int type, int format_id,
+			 const struct path *path)
+{
+	int err;
+	struct inode *inode;
+
+	err = dquot_quota_on(sb, type, format_id, path);
+	if (err)
+		return err;
+
+	inode = d_inode(path->dentry);
+	inode_lock(inode);
+	EXT2_I(inode)->i_flags |= EXT2_NOATIME_FL | EXT2_IMMUTABLE_FL;
+	inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
+			S_NOATIME | S_IMMUTABLE);
+	inode_unlock(inode);
+	mark_inode_dirty(inode);
+
+	return 0;
+}
+
+static int ext2_quota_off(struct super_block *sb, int type)
+{
+	struct inode *inode = sb_dqopt(sb)->files[type];
+	int err;
+
+	if (!inode || !igrab(inode))
+		goto out;
+
+	err = dquot_quota_off(sb, type);
+	if (err)
+		goto out_put;
+
+	inode_lock(inode);
+	EXT2_I(inode)->i_flags &= ~(EXT2_NOATIME_FL | EXT2_IMMUTABLE_FL);
+	inode_set_flags(inode, 0, S_NOATIME | S_IMMUTABLE);
+	inode_unlock(inode);
+	mark_inode_dirty(inode);
+out_put:
+	iput(inode);
+	return err;
+out:
+	return dquot_quota_off(sb, type);
+}
+
 #endif
 
 static struct file_system_type ext2_fs_type = {
-- 
2.12.0

[PATCH 04/11] jfs: Set flags on quota files directly

[Jan Kara]

Currently immutable and noatime flags on quota files are set by quota
code which requires us to copy inode->i_flags to our on disk version
of quota flags in GETFLAGS ioctl and copy_to_dinode(). Move to
setting / clearing these on-disk flags directly to save that copying.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jfs/super.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index c64c2574a0aa..e8aad7d87b8c 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -45,6 +45,7 @@
 #include "jfs_acl.h"
 #include "jfs_debug.h"
 #include "jfs_xattr.h"
+#include "jfs_dinode.h"
 
 MODULE_DESCRIPTION("The Journaled Filesystem (JFS)");
 MODULE_AUTHOR("Steve Best/Dave Kleikamp/Barry Arndt, IBM");
@@ -181,6 +182,35 @@ static int jfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
+#ifdef CONFIG_QUOTA
+static int jfs_quota_off(struct super_block *sb, int type);
+static int jfs_quota_on(struct super_block *sb, int type, int format_id,
+			const struct path *path);
+
+static void jfs_quota_off_umount(struct super_block *sb)
+{
+	int type;
+
+	for (type = 0; type < MAXQUOTAS; type++)
+		jfs_quota_off(sb, type);
+}
+
+static const struct quotactl_ops jfs_quotactl_ops = {
+	.quota_on	= jfs_quota_on,
+	.quota_off	= jfs_quota_off,
+	.quota_sync	= dquot_quota_sync,
+	.get_state	= dquot_get_state,
+	.set_info	= dquot_set_dqinfo,
+	.get_dqblk	= dquot_get_dqblk,
+	.set_dqblk	= dquot_set_dqblk,
+	.get_nextdqblk	= dquot_get_next_dqblk,
+};
+#else
+static inline void jfs_quota_off_umount(struct super_block *sb)
+{
+}
+#endif
+
 static void jfs_put_super(struct super_block *sb)
 {
 	struct jfs_sb_info *sbi = JFS_SBI(sb);
@@ -188,7 +218,7 @@ static void jfs_put_super(struct super_block *sb)
 
 	jfs_info("In jfs_put_super");
 
-	dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
+	jfs_quota_off_umount(sb);
 
 	rc = jfs_umount(sb);
 	if (rc)
@@ -536,7 +566,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_xattr = jfs_xattr_handlers;
 #ifdef CONFIG_QUOTA
 	sb->dq_op = &dquot_operations;
-	sb->s_qcop = &dquot_quotactl_ops;
+	sb->s_qcop = &jfs_quotactl_ops;
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
 #endif
 
@@ -840,6 +870,51 @@ static struct dquot **jfs_get_dquots(struct inode *inode)
 {
 	return JFS_IP(inode)->i_dquot;
 }
+
+static int jfs_quota_on(struct super_block *sb, int type, int format_id,
+			const struct path *path)
+{
+	int err;
+	struct inode *inode;
+
+	err = dquot_quota_on(sb, type, format_id, path);
+	if (err)
+		return err;
+
+	inode = d_inode(path->dentry);
+	inode_lock(inode);
+	JFS_IP(inode)->mode2 |= JFS_NOATIME_FL | JFS_IMMUTABLE_FL;
+	inode_set_flags(inode, S_NOATIME | S_IMMUTABLE,
+			S_NOATIME | S_IMMUTABLE);
+	inode_unlock(inode);
+	mark_inode_dirty(inode);
+
+	return 0;
+}
+
+static int jfs_quota_off(struct super_block *sb, int type)
+{
+	struct inode *inode = sb_dqopt(sb)->files[type];
+	int err;
+
+	if (!inode || !igrab(inode))
+		goto out;
+
+	err = dquot_quota_off(sb, type);
+	if (err)
+		goto out_put;
+
+	inode_lock(inode);
+	JFS_IP(inode)->mode2 &= ~(JFS_NOATIME_FL | JFS_IMMUTABLE_FL);
+	inode_set_flags(inode, 0, S_NOATIME | S_IMMUTABLE);
+	inode_unlock(inode);
+	mark_inode_dirty(inode);
+out_put:
+	iput(inode);
+	return err;
+out:
+	return dquot_quota_off(sb, type);
+}
 #endif
 
 static const struct super_operations jfs_super_operations = {
-- 
2.12.0

[PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags on quota files

[Jan Kara]

Currently we set IMMUTABLE and NOATIME flags on quota files to stop
userspace from messing with them. Now that all filesystems set these
flags in their quota_on handlers, we can stop setting the flags in
generic quota code. This will allow filesystems to stop copying i_flags
to their on-disk flags on various occasions.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 74b489e3714d..7e94cb0ecdde 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2188,8 +2188,7 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags)
 		/* This can happen when suspending quotas on remount-ro... */
 		if (toputinode[cnt] && !sb_has_quota_loaded(sb, cnt)) {
 			inode_lock(toputinode[cnt]);
-			toputinode[cnt]->i_flags &= ~(S_IMMUTABLE |
-				  S_NOATIME | S_NOQUOTA);
+			toputinode[cnt]->i_flags &= ~S_NOQUOTA;
 			truncate_inode_pages(&toputinode[cnt]->i_data, 0);
 			inode_unlock(toputinode[cnt]);
 			mark_inode_dirty_sync(toputinode[cnt]);
@@ -2237,7 +2236,6 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 	struct super_block *sb = inode->i_sb;
 	struct quota_info *dqopt = sb_dqopt(sb);
 	int error;
-	int oldflags = -1;
 
 	if (!fmt)
 		return -ESRCH;
@@ -2285,9 +2283,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 		 * possible) Also nobody should write to the file - we use
 		 * special IO operations which ignore the immutable bit. */
 		inode_lock(inode);
-		oldflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
-					     S_NOQUOTA);
-		inode->i_flags |= S_NOQUOTA | S_NOATIME | S_IMMUTABLE;
+		inode->i_flags |= S_NOQUOTA;
 		inode_unlock(inode);
 		/*
 		 * When S_NOQUOTA is set, remove dquot references as no more
@@ -2329,14 +2325,9 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 	dqopt->files[type] = NULL;
 	iput(inode);
 out_file_flags:
-	if (oldflags != -1) {
-		inode_lock(inode);
-		/* Set the flags back (in the case of accidental quotaon()
-		 * on a wrong file we don't want to mess up the flags) */
-		inode->i_flags &= ~(S_NOATIME | S_NOQUOTA | S_IMMUTABLE);
-		inode->i_flags |= oldflags;
-		inode_unlock(inode);
-	}
+	inode_lock(inode);
+	inode->i_flags &= ~S_NOQUOTA;
+	inode_unlock(inode);
 out_fmt:
 	put_quota_format(fmt);
 
-- 
2.12.0

Re: [PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags on quota files

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2756 bytes --]

On Apr 12, 2017, at 1:26 AM, Jan Kara <jack@suse.cz> wrote:
> 
> Currently we set IMMUTABLE and NOATIME flags on quota files to stop
> userspace from messing with them. Now that all filesystems set these
> flags in their quota_on handlers, we can stop setting the flags in
> generic quota code. This will allow filesystems to stop copying i_flags
> to their on-disk flags on various occasions.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/quota/dquot.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 74b489e3714d..7e94cb0ecdde 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2188,8 +2188,7 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags)
> 		/* This can happen when suspending quotas on remount-ro... */
> 		if (toputinode[cnt] && !sb_has_quota_loaded(sb, cnt)) {
> 			inode_lock(toputinode[cnt]);
> -			toputinode[cnt]->i_flags &= ~(S_IMMUTABLE |
> -				  S_NOATIME | S_NOQUOTA);
> +			toputinode[cnt]->i_flags &= ~S_NOQUOTA;
> 			truncate_inode_pages(&toputinode[cnt]->i_data, 0);
> 			inode_unlock(toputinode[cnt]);
> 			mark_inode_dirty_sync(toputinode[cnt]);
> @@ -2237,7 +2236,6 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
> 	struct super_block *sb = inode->i_sb;
> 	struct quota_info *dqopt = sb_dqopt(sb);
> 	int error;
> -	int oldflags = -1;
> 
> 	if (!fmt)
> 		return -ESRCH;
> @@ -2285,9 +2283,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
> 		 * possible) Also nobody should write to the file - we use
> 		 * special IO operations which ignore the immutable bit. */
> 		inode_lock(inode);
> -		oldflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
> -					     S_NOQUOTA);
> -		inode->i_flags |= S_NOQUOTA | S_NOATIME | S_IMMUTABLE;
> +		inode->i_flags |= S_NOQUOTA;
> 		inode_unlock(inode);
> 		/*
> 		 * When S_NOQUOTA is set, remove dquot references as no more
> @@ -2329,14 +2325,9 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
> 	dqopt->files[type] = NULL;
> 	iput(inode);
> out_file_flags:
> -	if (oldflags != -1) {
> -		inode_lock(inode);
> -		/* Set the flags back (in the case of accidental quotaon()
> -		 * on a wrong file we don't want to mess up the flags) */
> -		inode->i_flags &= ~(S_NOATIME | S_NOQUOTA | S_IMMUTABLE);
> -		inode->i_flags |= oldflags;
> -		inode_unlock(inode);
> -	}
> +	inode_lock(inode);
> +	inode->i_flags &= ~S_NOQUOTA;
> +	inode_unlock(inode);
> out_fmt:
> 	put_quota_format(fmt);
> 
> --
> 2.12.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 06/11] ext4: Remove ext4_get_inode_flags()

[Jan Kara]

Now that all places setting inode->i_flags that should be reflected in
on-disk flags are gone, we can remove ext4_get_inode_flags() call.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/ext4.h  |  1 -
 fs/ext4/inode.c | 26 --------------------------
 fs/ext4/ioctl.c |  2 --
 3 files changed, 29 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index f493af666591..3e8a03b88b3a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2476,7 +2476,6 @@ extern int ext4_truncate(struct inode *);
 extern int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length);
 extern int ext4_truncate_restart_trans(handle_t *, struct inode *, int nblocks);
 extern void ext4_set_inode_flags(struct inode *);
-extern void ext4_get_inode_flags(struct ext4_inode_info *);
 extern int ext4_alloc_da_blocks(struct inode *inode);
 extern void ext4_set_aops(struct inode *inode);
 extern int ext4_writepage_trans_blocks(struct inode *);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4247d8d25687..cfd8b29bbf92 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4502,31 +4502,6 @@ void ext4_set_inode_flags(struct inode *inode)
 			S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
 }
 
-/* Propagate flags from i_flags to EXT4_I(inode)->i_flags */
-void ext4_get_inode_flags(struct ext4_inode_info *ei)
-{
-	unsigned int vfs_fl;
-	unsigned long old_fl, new_fl;
-
-	do {
-		vfs_fl = ei->vfs_inode.i_flags;
-		old_fl = ei->i_flags;
-		new_fl = old_fl & ~(EXT4_SYNC_FL|EXT4_APPEND_FL|
-				EXT4_IMMUTABLE_FL|EXT4_NOATIME_FL|
-				EXT4_DIRSYNC_FL);
-		if (vfs_fl & S_SYNC)
-			new_fl |= EXT4_SYNC_FL;
-		if (vfs_fl & S_APPEND)
-			new_fl |= EXT4_APPEND_FL;
-		if (vfs_fl & S_IMMUTABLE)
-			new_fl |= EXT4_IMMUTABLE_FL;
-		if (vfs_fl & S_NOATIME)
-			new_fl |= EXT4_NOATIME_FL;
-		if (vfs_fl & S_DIRSYNC)
-			new_fl |= EXT4_DIRSYNC_FL;
-	} while (cmpxchg(&ei->i_flags, old_fl, new_fl) != old_fl);
-}
-
 static blkcnt_t ext4_inode_blocks(struct ext4_inode *raw_inode,
 				  struct ext4_inode_info *ei)
 {
@@ -4963,7 +4938,6 @@ static int ext4_do_update_inode(handle_t *handle,
 	if (ext4_test_inode_state(inode, EXT4_STATE_NEW))
 		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
 
-	ext4_get_inode_flags(ei);
 	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
 	i_uid = i_uid_read(inode);
 	i_gid = i_gid_read(inode);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a4273ddb9922..184e74eb3004 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -500,7 +500,6 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case EXT4_IOC_GETFLAGS:
-		ext4_get_inode_flags(ei);
 		flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
 		return put_user(flags, (int __user *) arg);
 	case EXT4_IOC_SETFLAGS: {
@@ -888,7 +887,6 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		struct fsxattr fa;
 
 		memset(&fa, 0, sizeof(struct fsxattr));
-		ext4_get_inode_flags(ei);
 		fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);
 
 		if (ext4_has_feature_project(inode->i_sb)) {
-- 
2.12.0

Re: [PATCH 06/11] ext4: Remove ext4_get_inode_flags()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 3472 bytes --]

On Apr 12, 2017, at 1:26 AM, Jan Kara <jack@suse.cz> wrote:
> 
> Now that all places setting inode->i_flags that should be reflected in
> on-disk flags are gone, we can remove ext4_get_inode_flags() call.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/ext4.h  |  1 -
> fs/ext4/inode.c | 26 --------------------------
> fs/ext4/ioctl.c |  2 --
> 3 files changed, 29 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index f493af666591..3e8a03b88b3a 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2476,7 +2476,6 @@ extern int ext4_truncate(struct inode *);
> extern int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length);
> extern int ext4_truncate_restart_trans(handle_t *, struct inode *, int nblocks);
> extern void ext4_set_inode_flags(struct inode *);
> -extern void ext4_get_inode_flags(struct ext4_inode_info *);
> extern int ext4_alloc_da_blocks(struct inode *inode);
> extern void ext4_set_aops(struct inode *inode);
> extern int ext4_writepage_trans_blocks(struct inode *);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4247d8d25687..cfd8b29bbf92 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4502,31 +4502,6 @@ void ext4_set_inode_flags(struct inode *inode)
> 			S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX);
> }
> 
> -/* Propagate flags from i_flags to EXT4_I(inode)->i_flags */
> -void ext4_get_inode_flags(struct ext4_inode_info *ei)
> -{
> -	unsigned int vfs_fl;
> -	unsigned long old_fl, new_fl;
> -
> -	do {
> -		vfs_fl = ei->vfs_inode.i_flags;
> -		old_fl = ei->i_flags;
> -		new_fl = old_fl & ~(EXT4_SYNC_FL|EXT4_APPEND_FL|
> -				EXT4_IMMUTABLE_FL|EXT4_NOATIME_FL|
> -				EXT4_DIRSYNC_FL);
> -		if (vfs_fl & S_SYNC)
> -			new_fl |= EXT4_SYNC_FL;
> -		if (vfs_fl & S_APPEND)
> -			new_fl |= EXT4_APPEND_FL;
> -		if (vfs_fl & S_IMMUTABLE)
> -			new_fl |= EXT4_IMMUTABLE_FL;
> -		if (vfs_fl & S_NOATIME)
> -			new_fl |= EXT4_NOATIME_FL;
> -		if (vfs_fl & S_DIRSYNC)
> -			new_fl |= EXT4_DIRSYNC_FL;
> -	} while (cmpxchg(&ei->i_flags, old_fl, new_fl) != old_fl);
> -}
> -
> static blkcnt_t ext4_inode_blocks(struct ext4_inode *raw_inode,
> 				  struct ext4_inode_info *ei)
> {
> @@ -4963,7 +4938,6 @@ static int ext4_do_update_inode(handle_t *handle,
> 	if (ext4_test_inode_state(inode, EXT4_STATE_NEW))
> 		memset(raw_inode, 0, EXT4_SB(inode->i_sb)->s_inode_size);
> 
> -	ext4_get_inode_flags(ei);
> 	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> 	i_uid = i_uid_read(inode);
> 	i_gid = i_gid_read(inode);
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index a4273ddb9922..184e74eb3004 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -500,7 +500,6 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> 
> 	switch (cmd) {
> 	case EXT4_IOC_GETFLAGS:
> -		ext4_get_inode_flags(ei);
> 		flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
> 		return put_user(flags, (int __user *) arg);
> 	case EXT4_IOC_SETFLAGS: {
> @@ -888,7 +887,6 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> 		struct fsxattr fa;
> 
> 		memset(&fa, 0, sizeof(struct fsxattr));
> -		ext4_get_inode_flags(ei);
> 		fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);
> 
> 		if (ext4_has_feature_project(inode->i_sb)) {
> --
> 2.12.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 07/11] ext2: Remove ext2_get_inode_flags()

[Jan Kara]

Now that all places setting inode->i_flags that should be reflected in
on-disk flags are gone, we can remove ext2_get_inode_flags() call.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/ext2.h  |  1 -
 fs/ext2/inode.c | 20 --------------------
 fs/ext2/ioctl.c |  1 -
 3 files changed, 22 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 5e64de9c5093..c7cfe9afa375 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -779,7 +779,6 @@ extern void ext2_evict_inode(struct inode *);
 extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int ext2_setattr (struct dentry *, struct iattr *);
 extern void ext2_set_inode_flags(struct inode *inode);
-extern void ext2_get_inode_flags(struct ext2_inode_info *);
 extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		       u64 start, u64 len);
 
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 128cce540645..1e2663fbbc54 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1384,25 +1384,6 @@ void ext2_set_inode_flags(struct inode *inode)
 		inode->i_flags |= S_DAX;
 }
 
-/* Propagate flags from i_flags to EXT2_I(inode)->i_flags */
-void ext2_get_inode_flags(struct ext2_inode_info *ei)
-{
-	unsigned int flags = ei->vfs_inode.i_flags;
-
-	ei->i_flags &= ~(EXT2_SYNC_FL|EXT2_APPEND_FL|
-			EXT2_IMMUTABLE_FL|EXT2_NOATIME_FL|EXT2_DIRSYNC_FL);
-	if (flags & S_SYNC)
-		ei->i_flags |= EXT2_SYNC_FL;
-	if (flags & S_APPEND)
-		ei->i_flags |= EXT2_APPEND_FL;
-	if (flags & S_IMMUTABLE)
-		ei->i_flags |= EXT2_IMMUTABLE_FL;
-	if (flags & S_NOATIME)
-		ei->i_flags |= EXT2_NOATIME_FL;
-	if (flags & S_DIRSYNC)
-		ei->i_flags |= EXT2_DIRSYNC_FL;
-}
-
 struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
 {
 	struct ext2_inode_info *ei;
@@ -1563,7 +1544,6 @@ static int __ext2_write_inode(struct inode *inode, int do_sync)
 	if (ei->i_state & EXT2_STATE_NEW)
 		memset(raw_inode, 0, EXT2_SB(sb)->s_inode_size);
 
-	ext2_get_inode_flags(ei);
 	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
 	if (!(test_opt(sb, NO_UID32))) {
 		raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
index 191e02b28ce8..087f122cca42 100644
--- a/fs/ext2/ioctl.c
+++ b/fs/ext2/ioctl.c
@@ -29,7 +29,6 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case EXT2_IOC_GETFLAGS:
-		ext2_get_inode_flags(ei);
 		flags = ei->i_flags & EXT2_FL_USER_VISIBLE;
 		return put_user(flags, (int __user *) arg);
 	case EXT2_IOC_SETFLAGS: {
-- 
2.12.0

Re: [PATCH 07/11] ext2: Remove ext2_get_inode_flags()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

On Apr 12, 2017, at 1:26 AM, Jan Kara <jack@suse.cz> wrote:
> 
> Now that all places setting inode->i_flags that should be reflected in
> on-disk flags are gone, we can remove ext2_get_inode_flags() call.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext2/ext2.h  |  1 -
> fs/ext2/inode.c | 20 --------------------
> fs/ext2/ioctl.c |  1 -
> 3 files changed, 22 deletions(-)
> 
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index 5e64de9c5093..c7cfe9afa375 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -779,7 +779,6 @@ extern void ext2_evict_inode(struct inode *);
> extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
> extern int ext2_setattr (struct dentry *, struct iattr *);
> extern void ext2_set_inode_flags(struct inode *inode);
> -extern void ext2_get_inode_flags(struct ext2_inode_info *);
> extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> 		       u64 start, u64 len);
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 128cce540645..1e2663fbbc54 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -1384,25 +1384,6 @@ void ext2_set_inode_flags(struct inode *inode)
> 		inode->i_flags |= S_DAX;
> }
> 
> -/* Propagate flags from i_flags to EXT2_I(inode)->i_flags */
> -void ext2_get_inode_flags(struct ext2_inode_info *ei)
> -{
> -	unsigned int flags = ei->vfs_inode.i_flags;
> -
> -	ei->i_flags &= ~(EXT2_SYNC_FL|EXT2_APPEND_FL|
> -			EXT2_IMMUTABLE_FL|EXT2_NOATIME_FL|EXT2_DIRSYNC_FL);
> -	if (flags & S_SYNC)
> -		ei->i_flags |= EXT2_SYNC_FL;
> -	if (flags & S_APPEND)
> -		ei->i_flags |= EXT2_APPEND_FL;
> -	if (flags & S_IMMUTABLE)
> -		ei->i_flags |= EXT2_IMMUTABLE_FL;
> -	if (flags & S_NOATIME)
> -		ei->i_flags |= EXT2_NOATIME_FL;
> -	if (flags & S_DIRSYNC)
> -		ei->i_flags |= EXT2_DIRSYNC_FL;
> -}
> -
> struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
> {
> 	struct ext2_inode_info *ei;
> @@ -1563,7 +1544,6 @@ static int __ext2_write_inode(struct inode *inode, int do_sync)
> 	if (ei->i_state & EXT2_STATE_NEW)
> 		memset(raw_inode, 0, EXT2_SB(sb)->s_inode_size);
> 
> -	ext2_get_inode_flags(ei);
> 	raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> 	if (!(test_opt(sb, NO_UID32))) {
> 		raw_inode->i_uid_low = cpu_to_le16(low_16_bits(uid));
> diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
> index 191e02b28ce8..087f122cca42 100644
> --- a/fs/ext2/ioctl.c
> +++ b/fs/ext2/ioctl.c
> @@ -29,7 +29,6 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> 
> 	switch (cmd) {
> 	case EXT2_IOC_GETFLAGS:
> -		ext2_get_inode_flags(ei);
> 		flags = ei->i_flags & EXT2_FL_USER_VISIBLE;
> 		return put_user(flags, (int __user *) arg);
> 	case EXT2_IOC_SETFLAGS: {
> --
> 2.12.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 08/11] jfs: Remove jfs_get_inode_flags()

[Jan Kara]

Now that all places setting inode->i_flags that should be reflected in
on-disk flags are gone, we can remove jfs_get_inode_flags() call.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jfs/ioctl.c     |  2 --
 fs/jfs/jfs_imap.c  |  1 -
 fs/jfs/jfs_inode.c | 18 ------------------
 fs/jfs/jfs_inode.h |  1 -
 4 files changed, 22 deletions(-)

diff --git a/fs/jfs/ioctl.c b/fs/jfs/ioctl.c
index fc89f9436784..5c5ac5b3aec3 100644
--- a/fs/jfs/ioctl.c
+++ b/fs/jfs/ioctl.c
@@ -64,7 +64,6 @@ long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case JFS_IOC_GETFLAGS:
-		jfs_get_inode_flags(jfs_inode);
 		flags = jfs_inode->mode2 & JFS_FL_USER_VISIBLE;
 		flags = jfs_map_ext2(flags, 0);
 		return put_user(flags, (int __user *) arg);
@@ -98,7 +97,6 @@ long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		/* Lock against other parallel changes of flags */
 		inode_lock(inode);
 
-		jfs_get_inode_flags(jfs_inode);
 		oldflags = jfs_inode->mode2;
 
 		/*
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 6aca224a5d68..f36ef68905a7 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -3148,7 +3148,6 @@ static void copy_to_dinode(struct dinode * dip, struct inode *ip)
 	else
 		dip->di_gid = cpu_to_le32(from_kgid(&init_user_ns,
 						    jfs_ip->saved_gid));
-	jfs_get_inode_flags(jfs_ip);
 	/*
 	 * mode2 is only needed for storing the higher order bits.
 	 * Trust i_mode for the lower order ones
diff --git a/fs/jfs/jfs_inode.c b/fs/jfs/jfs_inode.c
index 375dd257a34f..5e9b7bb3aabf 100644
--- a/fs/jfs/jfs_inode.c
+++ b/fs/jfs/jfs_inode.c
@@ -45,24 +45,6 @@ void jfs_set_inode_flags(struct inode *inode)
 			S_DIRSYNC | S_SYNC);
 }
 
-void jfs_get_inode_flags(struct jfs_inode_info *jfs_ip)
-{
-	unsigned int flags = jfs_ip->vfs_inode.i_flags;
-
-	jfs_ip->mode2 &= ~(JFS_IMMUTABLE_FL | JFS_APPEND_FL | JFS_NOATIME_FL |
-			   JFS_DIRSYNC_FL | JFS_SYNC_FL);
-	if (flags & S_IMMUTABLE)
-		jfs_ip->mode2 |= JFS_IMMUTABLE_FL;
-	if (flags & S_APPEND)
-		jfs_ip->mode2 |= JFS_APPEND_FL;
-	if (flags & S_NOATIME)
-		jfs_ip->mode2 |= JFS_NOATIME_FL;
-	if (flags & S_DIRSYNC)
-		jfs_ip->mode2 |= JFS_DIRSYNC_FL;
-	if (flags & S_SYNC)
-		jfs_ip->mode2 |= JFS_SYNC_FL;
-}
-
 /*
  * NAME:	ialloc()
  *
diff --git a/fs/jfs/jfs_inode.h b/fs/jfs/jfs_inode.h
index 9271cfe4a149..7b0b3a40788f 100644
--- a/fs/jfs/jfs_inode.h
+++ b/fs/jfs/jfs_inode.h
@@ -33,7 +33,6 @@ extern void jfs_truncate(struct inode *);
 extern void jfs_truncate_nolock(struct inode *, loff_t);
 extern void jfs_free_zero_link(struct inode *);
 extern struct dentry *jfs_get_parent(struct dentry *dentry);
-extern void jfs_get_inode_flags(struct jfs_inode_info *);
 extern struct dentry *jfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
 	int fh_len, int fh_type);
 extern struct dentry *jfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-- 
2.12.0

[PATCH 09/11] reiserfs: Remove useless setting of i_flags

[Jan Kara]

reiserfs_new_inode() clears IMMUTABLE and APPEND flags from a symlink
i_flags however a few lines below in sd_attrs_to_i_attrs() we will
happily overwrite i_flags with whatever we inherited from the directory.
Since this behavior is there for ages just remove the useless setting of
i_flags.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/reiserfs/inode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index a6ab9d64ea1b..aeca1a0b7b5a 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -2002,10 +2002,6 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 
 	/* uid and gid must already be set by the caller for quota init */
 
-	/* symlink cannot be immutable or append only, right? */
-	if (S_ISLNK(inode->i_mode))
-		inode->i_flags &= ~(S_IMMUTABLE | S_APPEND);
-
 	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
 	inode->i_size = i_size;
 	inode->i_blocks = 0;
-- 
2.12.0

[PATCH 10/11] reiserfs: Remove i_attrs_to_sd_attrs()

[Jan Kara]

Now that all places setting inode->i_flags that should be reflected in
on-disk flags are gone, we can remove i_attrs_to_sd_attrs() call.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/reiserfs/inode.c    | 27 +--------------------------
 fs/reiserfs/ioctl.c    |  1 -
 fs/reiserfs/reiserfs.h |  1 -
 3 files changed, 1 insertion(+), 28 deletions(-)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index aeca1a0b7b5a..873fc04e9403 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1375,7 +1375,6 @@ static void init_inode(struct inode *inode, struct treepath *path)
 static void inode2sd(void *sd, struct inode *inode, loff_t size)
 {
 	struct stat_data *sd_v2 = (struct stat_data *)sd;
-	__u16 flags;
 
 	set_sd_v2_mode(sd_v2, inode->i_mode);
 	set_sd_v2_nlink(sd_v2, inode->i_nlink);
@@ -1390,9 +1389,7 @@ static void inode2sd(void *sd, struct inode *inode, loff_t size)
 		set_sd_v2_rdev(sd_v2, new_encode_dev(inode->i_rdev));
 	else
 		set_sd_v2_generation(sd_v2, inode->i_generation);
-	flags = REISERFS_I(inode)->i_attrs;
-	i_attrs_to_sd_attrs(inode, &flags);
-	set_sd_v2_attrs(sd_v2, flags);
+	set_sd_v2_attrs(sd_v2, REISERFS_I(inode)->i_attrs);
 }
 
 /* used to copy inode's fields to old stat data */
@@ -3091,28 +3088,6 @@ void sd_attrs_to_i_attrs(__u16 sd_attrs, struct inode *inode)
 	}
 }
 
-void i_attrs_to_sd_attrs(struct inode *inode, __u16 * sd_attrs)
-{
-	if (reiserfs_attrs(inode->i_sb)) {
-		if (inode->i_flags & S_IMMUTABLE)
-			*sd_attrs |= REISERFS_IMMUTABLE_FL;
-		else
-			*sd_attrs &= ~REISERFS_IMMUTABLE_FL;
-		if (inode->i_flags & S_SYNC)
-			*sd_attrs |= REISERFS_SYNC_FL;
-		else
-			*sd_attrs &= ~REISERFS_SYNC_FL;
-		if (inode->i_flags & S_NOATIME)
-			*sd_attrs |= REISERFS_NOATIME_FL;
-		else
-			*sd_attrs &= ~REISERFS_NOATIME_FL;
-		if (REISERFS_I(inode)->i_flags & i_nopack_mask)
-			*sd_attrs |= REISERFS_NOTAIL_FL;
-		else
-			*sd_attrs &= ~REISERFS_NOTAIL_FL;
-	}
-}
-
 /*
  * decide if this buffer needs to stay around for data logging or ordered
  * write purposes
diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c
index 1f4692a505a0..acbbaf7a0bb2 100644
--- a/fs/reiserfs/ioctl.c
+++ b/fs/reiserfs/ioctl.c
@@ -47,7 +47,6 @@ long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		}
 
 		flags = REISERFS_I(inode)->i_attrs;
-		i_attrs_to_sd_attrs(inode, (__u16 *) & flags);
 		err = put_user(flags, (int __user *)arg);
 		break;
 	case REISERFS_IOC_SETFLAGS:{
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index 2adcde137c3f..784b32568497 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -3099,7 +3099,6 @@ static inline void reiserfs_update_sd(struct reiserfs_transaction_handle *th,
 }
 
 void sd_attrs_to_i_attrs(__u16 sd_attrs, struct inode *inode);
-void i_attrs_to_sd_attrs(struct inode *inode, __u16 * sd_attrs);
 int reiserfs_setattr(struct dentry *dentry, struct iattr *attr);
 
 int __reiserfs_write_begin(struct page *page, unsigned from, unsigned len);
-- 
2.12.0

[PATCH 11/11] quota: Remove dquot_quotactl_ops

[Jan Kara]

Nobody uses them anymore.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/dquot.c         | 12 ------------
 include/linux/quotaops.h |  1 -
 2 files changed, 13 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 7e94cb0ecdde..ebf80c7739e1 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2771,18 +2771,6 @@ int dquot_set_dqinfo(struct super_block *sb, int type, struct qc_info *ii)
 }
 EXPORT_SYMBOL(dquot_set_dqinfo);
 
-const struct quotactl_ops dquot_quotactl_ops = {
-	.quota_on	= dquot_quota_on,
-	.quota_off	= dquot_quota_off,
-	.quota_sync	= dquot_quota_sync,
-	.get_state	= dquot_get_state,
-	.set_info	= dquot_set_dqinfo,
-	.get_dqblk	= dquot_get_dqblk,
-	.get_nextdqblk	= dquot_get_next_dqblk,
-	.set_dqblk	= dquot_set_dqblk
-};
-EXPORT_SYMBOL(dquot_quotactl_ops);
-
 const struct quotactl_ops dquot_quotactl_sysfile_ops = {
 	.quota_enable	= dquot_quota_enable,
 	.quota_disable	= dquot_quota_disable,
diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
index 799a63d0e1a8..9c6f768b7d32 100644
--- a/include/linux/quotaops.h
+++ b/include/linux/quotaops.h
@@ -162,7 +162,6 @@ static inline bool sb_has_quota_active(struct super_block *sb, int type)
  * Operations supported for diskquotas.
  */
 extern const struct dquot_operations dquot_operations;
-extern const struct quotactl_ops dquot_quotactl_ops;
 extern const struct quotactl_ops dquot_quotactl_sysfile_ops;
 
 #else
-- 
2.12.0

Can the patch set [1] be put up on openSUSE Build Service as well please ?

[doiggl]

Hello,
Can the patch set [1] be put up on openSUSE Build Service as well please ?
 
- check it builds.

example:
kernel-vanilla
https://build.opensuse.org/package/show/Kernel:vanilla/kernel-vanilla#
https://build.opensuse.org/package/show/Kernel:vanilla/kernel-source

Thanks
--Glenn

From:
http://www.spinics.net/lists/reiserfs-devel/

[PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly
http://www.spinics.net/lists/reiserfs-devel/msg05442.html

[1]
[PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly, Jan
Kara

    [PATCH 04/11] jfs: Set flags on quota files directly, Jan Kara
    [PATCH 08/11] jfs: Remove jfs_get_inode_flags(), Jan Kara
    [PATCH 07/11] ext2: Remove ext2_get_inode_flags(), Jan Kara
        Re: [PATCH 07/11] ext2: Remove ext2_get_inode_flags(), Andreas
Dilger
    [PATCH 06/11] ext4: Remove ext4_get_inode_flags(), Jan Kara
        Re: [PATCH 06/11] ext4: Remove ext4_get_inode_flags(), Andreas
Dilger
    [PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags on quota
files, Jan Kara
        Re: [PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags
on quota files, Andreas Dilger
    [PATCH 11/11] quota: Remove dquot_quotactl_ops, Jan Kara
    [PATCH 09/11] reiserfs: Remove useless setting of i_flags, Jan Kara
    [PATCH 10/11] reiserfs: Remove i_attrs_to_sd_attrs(), Jan Kara
    [PATCH 01/11] ext4: Set flags on quota files directly, Jan Kara
        Re: [PATCH 01/11] ext4: Set flags on quota files directly, Andreas
Dilger
            Re: [PATCH 01/11] ext4: Set flags on quota files directly, Jan
Kara
    [PATCH 03/11] ext2: Set flags on quota files directly, Jan Kara
    [PATCH 02/11] reiserfs: Set flags on quota files directly, Jan Kara

[PATCH 1/2] reiserfs: Make cancel_old_flush() reliable, Jan Kara

    [PATCH 2/2] reiserfs: Protect dquot_writeback_dquots() by s_umount
semaphore, Jan Kara

https://build.opensuse.org/package/show/Kernel:vanilla/kernel-source

Re: Can the patch set [1] be put up on openSUSE Build Service as well please ?

[Jan Kara]

Hello,

On Sat 15-04-17 13:22:51, doiggl@velocitynet.com.au wrote:
> Can the patch set [1] be put up on openSUSE Build Service as well please ?

I guess I could create a kernel branch with these patches in my home project
but why?

								Honza

> - check it builds.
> 
> example:
> kernel-vanilla
> https://build.opensuse.org/package/show/Kernel:vanilla/kernel-vanilla#
> https://build.opensuse.org/package/show/Kernel:vanilla/kernel-source
> 
> Thanks
> --Glenn
> 
> From:
> http://www.spinics.net/lists/reiserfs-devel/
> 
> [PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly
> http://www.spinics.net/lists/reiserfs-devel/msg05442.html
> 
> [1]
> [PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly, Jan
> Kara
> 
>     [PATCH 04/11] jfs: Set flags on quota files directly, Jan Kara
>     [PATCH 08/11] jfs: Remove jfs_get_inode_flags(), Jan Kara
>     [PATCH 07/11] ext2: Remove ext2_get_inode_flags(), Jan Kara
>         Re: [PATCH 07/11] ext2: Remove ext2_get_inode_flags(), Andreas
> Dilger
>     [PATCH 06/11] ext4: Remove ext4_get_inode_flags(), Jan Kara
>         Re: [PATCH 06/11] ext4: Remove ext4_get_inode_flags(), Andreas
> Dilger
>     [PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags on quota
> files, Jan Kara
>         Re: [PATCH 05/11] quota: Stop setting IMMUTABLE and NOATIME flags
> on quota files, Andreas Dilger
>     [PATCH 11/11] quota: Remove dquot_quotactl_ops, Jan Kara
>     [PATCH 09/11] reiserfs: Remove useless setting of i_flags, Jan Kara
>     [PATCH 10/11] reiserfs: Remove i_attrs_to_sd_attrs(), Jan Kara
>     [PATCH 01/11] ext4: Set flags on quota files directly, Jan Kara
>         Re: [PATCH 01/11] ext4: Set flags on quota files directly, Andreas
> Dilger
>             Re: [PATCH 01/11] ext4: Set flags on quota files directly, Jan
> Kara
>     [PATCH 03/11] ext2: Set flags on quota files directly, Jan Kara
>     [PATCH 02/11] reiserfs: Set flags on quota files directly, Jan Kara
> 
> [PATCH 1/2] reiserfs: Make cancel_old_flush() reliable, Jan Kara
> 
>     [PATCH 2/2] reiserfs: Protect dquot_writeback_dquots() by s_umount
> semaphore, Jan Kara
> 
> https://build.opensuse.org/package/show/Kernel:vanilla/kernel-source
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/11] quota: Stop setting IMMUTABLE and NOATIME flags directly

[Jan Kara]

On Wed 12-04-17 09:26:00, Jan Kara wrote:
> Hello,
> 
> this series removes setting of IMMUTABLE and NOATIME flags from generic quota
> code and moves it to filesystem's quota_on / quota_off handlers. Since this
> was the only place where generic code was modifying inode flags that need to
> be persistent on disk, we can remove functions syncing inode->i_flags to
> on-disk flags after this. This was motivated by recent statx() work that
> wants consistent view of inode flags without holding i_mutex.
> 
> Patches passed basic testing for ext2, ext4, reiserfs, compile-tested for jfs
> only. Please review, patches are mostly trivial but I could have missed
> something. If people are fine with the changes, I plan to merge them through
> my tree.

Since there were no objections to the patches I have pulled them into my
tree and will push them to Linus during the merge window.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR