From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751763AbdFTP2U (ORCPT ); Tue, 20 Jun 2017 11:28:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:54986 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750993AbdFTP2T (ORCPT ); Tue, 20 Jun 2017 11:28:19 -0400 Date: Tue, 20 Jun 2017 17:28:16 +0200 From: Jan Kara To: Tahsin Erdogan Cc: Jan Kara , linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 28/31] quota: add get_inode_usage callback to transfer multi-inode charges Message-ID: <20170620152816.GA31922@quack2.suse.cz> References: <20170619123614.GA20405@quack2.suse.cz> <20170620091210.13343-1-tahsin@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170620091210.13343-1-tahsin@google.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 The patch looks good to me. Feel free to add: Acked-by: Jan Kara 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 SUSE Labs, CR