[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170620152816.GA31922@quack2.suse.cz>
Date: Tue, 20 Jun 2017 17:28:16 +0200
From: Jan Kara <jack@...e.cz>
To: Tahsin Erdogan <tahsin@...gle.com>
Cc: Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 28/31] quota: add get_inode_usage callback to transfer
multi-inode charges
On Tue 20-06-17 02:12:10, Tahsin Erdogan wrote:
> Ext4 ea_inode feature allows storing xattr values in external inodes to
> be able to store values that are bigger than a block in size. Ext4 also
> has deduplication support for these type of inodes. With deduplication,
> the actual storage waste is eliminated but the users of such inodes are
> still charged full quota for the inodes as if there was no sharing
> happening in the background.
>
> This design requires ext4 to manually charge the users because the
> inodes are shared.
>
> An implication of this is that, if someone calls chown on a file that
> has such references we need to transfer the quota for the file and xattr
> inodes. Current dquot_transfer() function implicitly transfers one inode
> charge. With ea_inode feature, we would like to transfer multiple inode
> charges.
>
> Add get_inode_usage callback which can interrogate the total number of
> inodes that were charged for a given inode.
>
> Signed-off-by: Tahsin Erdogan <tahsin@...gle.com>
The patch looks good to me. Feel free to add:
Acked-by: Jan Kara <jack@...e.cz>
Honza
> ---
> v2:
> - added get_inode_usage() callback to query total inodes charge to
> be transferred
>
> fs/ext4/inode.c | 7 +++++++
> fs/ext4/ioctl.c | 6 ++++++
> fs/ext4/super.c | 21 ++++++++++----------
> fs/ext4/xattr.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/ext4/xattr.h | 2 ++
> fs/quota/dquot.c | 16 +++++++++++----
> include/linux/quota.h | 2 ++
> 7 files changed, 94 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ea95bd9eab81..cd22de0b5d2c 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5295,7 +5295,14 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
> error = PTR_ERR(handle);
> goto err_out;
> }
> +
> + /* dquot_transfer() calls back ext4_get_inode_usage() which
> + * counts xattr inode references.
> + */
> + down_read(&EXT4_I(inode)->xattr_sem);
> error = dquot_transfer(inode, attr);
> + up_read(&EXT4_I(inode)->xattr_sem);
> +
> if (error) {
> ext4_journal_stop(handle);
> return error;
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index dde8deb11e59..42b3a73143cf 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -373,7 +373,13 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>
> transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid));
> if (!IS_ERR(transfer_to[PRJQUOTA])) {
> +
> + /* __dquot_transfer() calls back ext4_get_inode_usage() which
> + * counts xattr inode references.
> + */
> + down_read(&EXT4_I(inode)->xattr_sem);
> err = __dquot_transfer(inode, transfer_to);
> + up_read(&EXT4_I(inode)->xattr_sem);
> dqput(transfer_to[PRJQUOTA]);
> if (err)
> goto out_dirty;
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 2bfacd737bb6..4b15bf674d45 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1263,16 +1263,17 @@ static struct dquot **ext4_get_dquots(struct inode *inode)
> }
>
> static const struct dquot_operations ext4_quota_operations = {
> - .get_reserved_space = ext4_get_reserved_space,
> - .write_dquot = ext4_write_dquot,
> - .acquire_dquot = ext4_acquire_dquot,
> - .release_dquot = ext4_release_dquot,
> - .mark_dirty = ext4_mark_dquot_dirty,
> - .write_info = ext4_write_info,
> - .alloc_dquot = dquot_alloc,
> - .destroy_dquot = dquot_destroy,
> - .get_projid = ext4_get_projid,
> - .get_next_id = ext4_get_next_id,
> + .get_reserved_space = ext4_get_reserved_space,
> + .write_dquot = ext4_write_dquot,
> + .acquire_dquot = ext4_acquire_dquot,
> + .release_dquot = ext4_release_dquot,
> + .mark_dirty = ext4_mark_dquot_dirty,
> + .write_info = ext4_write_info,
> + .alloc_dquot = dquot_alloc,
> + .destroy_dquot = dquot_destroy,
> + .get_projid = ext4_get_projid,
> + .get_inode_usage = ext4_get_inode_usage,
> + .get_next_id = ext4_get_next_id,
> };
>
> static const struct quotactl_ops ext4_qctl_operations = {
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index d7e60358ec91..5e20f29afe9e 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -734,6 +734,60 @@ static void ext4_xattr_update_super_block(handle_t *handle,
> }
> }
>
> +int ext4_get_inode_usage(struct inode *inode, qsize_t *usage)
> +{
> + struct ext4_iloc iloc = { .bh = NULL };
> + struct buffer_head *bh = NULL;
> + struct ext4_inode *raw_inode;
> + struct ext4_xattr_ibody_header *header;
> + struct ext4_xattr_entry *entry;
> + qsize_t ea_inode_refs = 0;
> + void *end;
> + int ret;
> +
> + lockdep_assert_held_read(&EXT4_I(inode)->xattr_sem);
> +
> + if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
> + ret = ext4_get_inode_loc(inode, &iloc);
> + if (ret)
> + goto out;
> + raw_inode = ext4_raw_inode(&iloc);
> + header = IHDR(inode, raw_inode);
> + end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
> + ret = xattr_check_inode(inode, header, end);
> + if (ret)
> + goto out;
> +
> + for (entry = IFIRST(header); !IS_LAST_ENTRY(entry);
> + entry = EXT4_XATTR_NEXT(entry))
> + if (entry->e_value_inum)
> + ea_inode_refs++;
> + }
> +
> + if (EXT4_I(inode)->i_file_acl) {
> + bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
> + if (!bh) {
> + ret = -EIO;
> + goto out;
> + }
> +
> + if (ext4_xattr_check_block(inode, bh)) {
> + ret = -EFSCORRUPTED;
> + goto out;
> + }
> +
> + for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry);
> + entry = EXT4_XATTR_NEXT(entry))
> + if (entry->e_value_inum)
> + ea_inode_refs++;
> + }
> + *usage = ea_inode_refs + 1;
> +out:
> + brelse(iloc.bh);
> + brelse(bh);
> + return ret;
> +}
> +
> static inline size_t round_up_cluster(struct inode *inode, size_t length)
> {
> struct super_block *sb = inode->i_sb;
> diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
> index 67616cb9a059..26119a67c8c3 100644
> --- a/fs/ext4/xattr.h
> +++ b/fs/ext4/xattr.h
> @@ -193,3 +193,5 @@ extern void ext4_xattr_inode_set_class(struct inode *ea_inode);
> #else
> static inline void ext4_xattr_inode_set_class(struct inode *ea_inode) { }
> #endif
> +
> +extern int ext4_get_inode_usage(struct inode *inode, qsize_t *usage);
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 48813aeaab80..53a17496c5c5 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1910,6 +1910,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> {
> qsize_t space, cur_space;
> qsize_t rsv_space = 0;
> + qsize_t inode_usage = 1;
> struct dquot *transfer_from[MAXQUOTAS] = {};
> int cnt, ret = 0;
> char is_valid[MAXQUOTAS] = {};
> @@ -1919,6 +1920,13 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
>
> if (IS_NOQUOTA(inode))
> return 0;
> +
> + if (inode->i_sb->dq_op->get_inode_usage) {
> + ret = inode->i_sb->dq_op->get_inode_usage(inode, &inode_usage);
> + if (ret)
> + return ret;
> + }
> +
> /* Initialize the arrays */
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> warn_to[cnt].w_type = QUOTA_NL_NOWARN;
> @@ -1946,7 +1954,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> continue;
> is_valid[cnt] = 1;
> transfer_from[cnt] = i_dquot(inode)[cnt];
> - ret = check_idq(transfer_to[cnt], 1, &warn_to[cnt]);
> + ret = check_idq(transfer_to[cnt], inode_usage, &warn_to[cnt]);
> if (ret)
> goto over_quota;
> ret = check_bdq(transfer_to[cnt], space, 0, &warn_to[cnt]);
> @@ -1963,7 +1971,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> /* Due to IO error we might not have transfer_from[] structure */
> if (transfer_from[cnt]) {
> int wtype;
> - wtype = info_idq_free(transfer_from[cnt], 1);
> + wtype = info_idq_free(transfer_from[cnt], inode_usage);
> if (wtype != QUOTA_NL_NOWARN)
> prepare_warning(&warn_from_inodes[cnt],
> transfer_from[cnt], wtype);
> @@ -1971,13 +1979,13 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
> if (wtype != QUOTA_NL_NOWARN)
> prepare_warning(&warn_from_space[cnt],
> transfer_from[cnt], wtype);
> - dquot_decr_inodes(transfer_from[cnt], 1);
> + dquot_decr_inodes(transfer_from[cnt], inode_usage);
> dquot_decr_space(transfer_from[cnt], cur_space);
> dquot_free_reserved_space(transfer_from[cnt],
> rsv_space);
> }
>
> - dquot_incr_inodes(transfer_to[cnt], 1);
> + dquot_incr_inodes(transfer_to[cnt], inode_usage);
> dquot_incr_space(transfer_to[cnt], cur_space);
> dquot_resv_space(transfer_to[cnt], rsv_space);
>
> diff --git a/include/linux/quota.h b/include/linux/quota.h
> index 3434eef2a5aa..bfd077ca6ac3 100644
> --- a/include/linux/quota.h
> +++ b/include/linux/quota.h
> @@ -332,6 +332,8 @@ struct dquot_operations {
> * quota code only */
> qsize_t *(*get_reserved_space) (struct inode *);
> int (*get_projid) (struct inode *, kprojid_t *);/* Get project ID */
> + /* Get number of inodes that were charged for a given inode */
> + int (*get_inode_usage) (struct inode *, qsize_t *);
> /* Get next ID with active quota structure */
> int (*get_next_id) (struct super_block *sb, struct kqid *qid);
> };
> --
> 2.13.1.518.g3df882009-goog
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists