| Message ID | 20170531081517.11438-28-tahsin@google.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed 31-05-17 01:15:17, 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. In our case, we would like to specify additional inodes to be > transferred. Hum, rather handle this similarly to how we handle delalloc reserved space. Add a callback to dq_ops to get "inode usage" of an inode and then use it in dquot_transfer(), dquot_free_inode(), dquot_alloc_inode(). Honza > Signed-off-by: Tahsin Erdogan <tahsin@google.com> > --- > fs/ext2/inode.c | 2 +- > fs/ext4/inode.c | 8 ++++++- > fs/ext4/ioctl.c | 13 +++++++++++- > fs/ext4/xattr.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ > fs/ext4/xattr.h | 2 ++ > fs/jfs/file.c | 2 +- > fs/ocfs2/file.c | 2 +- > fs/quota/dquot.c | 16 +++++++------- > fs/reiserfs/inode.c | 2 +- > include/linux/quotaops.h | 8 ++++--- > 10 files changed, 93 insertions(+), 16 deletions(-) > > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c > index 2dcbd5698884..a13ba5dcb355 100644 > --- a/fs/ext2/inode.c > +++ b/fs/ext2/inode.c > @@ -1656,7 +1656,7 @@ int ext2_setattr(struct dentry *dentry, struct iattr *iattr) > } > if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) || > (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) { > - error = dquot_transfer(inode, iattr); > + error = dquot_transfer(inode, iattr, 0); > if (error) > return error; > } > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 6f5872197d6c..28abbbdbbb80 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -5267,6 +5267,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) > int error, rc = 0; > int orphan = 0; > const unsigned int ia_valid = attr->ia_valid; > + int ea_inode_refs; > > if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) > return -EIO; > @@ -5293,7 +5294,12 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) > error = PTR_ERR(handle); > goto err_out; > } > - error = dquot_transfer(inode, attr); > + > + down_read(&EXT4_I(inode)->xattr_sem); > + error = ea_inode_refs = ext4_xattr_inode_count(inode); > + if (ea_inode_refs >= 0) > + error = dquot_transfer(inode, attr, ea_inode_refs); > + 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..9938dc8e24c8 100644 > --- a/fs/ext4/ioctl.c > +++ b/fs/ext4/ioctl.c > @@ -21,6 +21,7 @@ > #include "ext4.h" > #include <linux/fsmap.h> > #include "fsmap.h" > +#include "xattr.h" > #include <trace/events/ext4.h> > > /** > @@ -319,6 +320,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) > struct ext4_iloc iloc; > struct ext4_inode *raw_inode; > struct dquot *transfer_to[MAXQUOTAS] = { }; > + int ea_inode_refs; > > if (!ext4_has_feature_project(sb)) { > if (projid != EXT4_DEF_PROJID) > @@ -371,9 +373,17 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) > if (err) > goto out_stop; > > + down_read(&EXT4_I(inode)->xattr_sem); > + ea_inode_refs = ext4_xattr_inode_count(inode); > + if (ea_inode_refs < 0) { > + up_read(&EXT4_I(inode)->xattr_sem); > + err = ea_inode_refs; > + goto out_stop; > + } > + > transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid)); > if (!IS_ERR(transfer_to[PRJQUOTA])) { > - err = __dquot_transfer(inode, transfer_to); > + err = __dquot_transfer(inode, transfer_to, ea_inode_refs); > dqput(transfer_to[PRJQUOTA]); > if (err) > goto out_dirty; > @@ -382,6 +392,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) > EXT4_I(inode)->i_projid = kprojid; > inode->i_ctime = current_time(inode); > out_dirty: > + up_read(&EXT4_I(inode)->xattr_sem); > rc = ext4_mark_iloc_dirty(handle, inode, &iloc); > if (!err) > err = rc; > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c > index caddc176a612..1d6fcbb01517 100644 > --- a/fs/ext4/xattr.c > +++ b/fs/ext4/xattr.c > @@ -701,6 +701,60 @@ static void ext4_xattr_update_super_block(handle_t *handle, > } > } > > +int ext4_xattr_inode_count(struct inode *inode) > +{ > + 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; > + int inode_count = 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) > + inode_count++; > + } > + > + 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) > + inode_count++; > + } > + ret = inode_count; > +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..8ef6fe123255 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 > + > +int ext4_xattr_inode_count(struct inode *inode); > diff --git a/fs/jfs/file.c b/fs/jfs/file.c > index 739492c7a3fd..b08e0b0449a7 100644 > --- a/fs/jfs/file.c > +++ b/fs/jfs/file.c > @@ -114,7 +114,7 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr) > } > if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) || > (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) { > - rc = dquot_transfer(inode, iattr); > + rc = dquot_transfer(inode, iattr, 0); > if (rc) > return rc; > } > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c > index bfeb647459d9..d3cbf6467af6 100644 > --- a/fs/ocfs2/file.c > +++ b/fs/ocfs2/file.c > @@ -1259,7 +1259,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) > mlog_errno(status); > goto bail_unlock; > } > - status = __dquot_transfer(inode, transfer_to); > + status = __dquot_transfer(inode, transfer_to, 0); > if (status < 0) > goto bail_commit; > } else { > diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > index 48813aeaab80..16e13d554aaa 100644 > --- a/fs/quota/dquot.c > +++ b/fs/quota/dquot.c > @@ -1906,10 +1906,12 @@ EXPORT_SYMBOL(dquot_free_inode); > * We are holding reference on transfer_from & transfer_to, no need to > * protect them by srcu_read_lock(). > */ > -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to) > +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to, > + int inodes_extra) > { > qsize_t space, cur_space; > qsize_t rsv_space = 0; > + qsize_t inode_count = 1 + inodes_extra; > struct dquot *transfer_from[MAXQUOTAS] = {}; > int cnt, ret = 0; > char is_valid[MAXQUOTAS] = {}; > @@ -1946,7 +1948,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_count, &warn_to[cnt]); > if (ret) > goto over_quota; > ret = check_bdq(transfer_to[cnt], space, 0, &warn_to[cnt]); > @@ -1963,7 +1965,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_count); > if (wtype != QUOTA_NL_NOWARN) > prepare_warning(&warn_from_inodes[cnt], > transfer_from[cnt], wtype); > @@ -1971,13 +1973,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_count); > 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_count); > dquot_incr_space(transfer_to[cnt], cur_space); > dquot_resv_space(transfer_to[cnt], rsv_space); > > @@ -2005,7 +2007,7 @@ EXPORT_SYMBOL(__dquot_transfer); > /* Wrapper for transferring ownership of an inode for uid/gid only > * Called from FSXXX_setattr() > */ > -int dquot_transfer(struct inode *inode, struct iattr *iattr) > +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra) > { > struct dquot *transfer_to[MAXQUOTAS] = {}; > struct dquot *dquot; > @@ -2037,7 +2039,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr) > } > transfer_to[GRPQUOTA] = dquot; > } > - ret = __dquot_transfer(inode, transfer_to); > + ret = __dquot_transfer(inode, transfer_to, inodes_extra); > out_put: > dqput_all(transfer_to); > return ret; > diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c > index 873fc04e9403..51586051b5dd 100644 > --- a/fs/reiserfs/inode.c > +++ b/fs/reiserfs/inode.c > @@ -3370,7 +3370,7 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr) > reiserfs_write_unlock(inode->i_sb); > if (error) > goto out; > - error = dquot_transfer(inode, attr); > + error = dquot_transfer(inode, attr, 0); > reiserfs_write_lock(inode->i_sb); > if (error) { > journal_end(&th); > diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h > index dda22f45fc1b..b7bcd9c6db6c 100644 > --- a/include/linux/quotaops.h > +++ b/include/linux/quotaops.h > @@ -106,8 +106,9 @@ int dquot_get_next_dqblk(struct super_block *sb, struct kqid *id, > int dquot_set_dqblk(struct super_block *sb, struct kqid id, > struct qc_dqblk *di); > > -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to); > -int dquot_transfer(struct inode *inode, struct iattr *iattr); > +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to, > + int inodes_extra); > +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra); > > static inline struct mem_dqinfo *sb_dqinfo(struct super_block *sb, int type) > { > @@ -226,7 +227,8 @@ static inline void dquot_free_inode(struct inode *inode) > { > } > > -static inline int dquot_transfer(struct inode *inode, struct iattr *iattr) > +static inline int dquot_transfer(struct inode *inode, struct iattr *iattr, > + int inodes_extra) > { > return 0; > } > -- > 2.13.0.219.gdb65acc882-goog > >
On Thu, Jun 15, 2017 at 12:57 AM, Jan Kara <jack@suse.cz> wrote: > Hum, rather handle this similarly to how we handle delalloc reserved space. > Add a callback to dq_ops to get "inode usage" of an inode and then use it > in dquot_transfer(), dquot_free_inode(), dquot_alloc_inode(). I tried that approach by adding a "int get_inode_usage(struct inode *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, ext4 code that calculates the number of internal inodes (ext4_xattr_inode_count()) is subject to failures so the callback has to be able to report errors. And, that itself is problematic because we can't afford to have errors in dquot_free_inode(). If you have thoughts about how to address this please let me know. Alternatively, I could try to make this patch less intrusive by keeping the existing dquot_transfer() signature and add a new dquot_transfer_usage() that accepts inode_usage as a parameter. What do you think?
On Fri 16-06-17 18:50:58, Tahsin Erdogan wrote: > On Thu, Jun 15, 2017 at 12:57 AM, Jan Kara <jack@suse.cz> wrote: > > Hum, rather handle this similarly to how we handle delalloc reserved space. > > Add a callback to dq_ops to get "inode usage" of an inode and then use it > > in dquot_transfer(), dquot_free_inode(), dquot_alloc_inode(). > > I tried that approach by adding a "int get_inode_usage(struct inode > *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, > ext4 code that calculates the number of internal inodes > (ext4_xattr_inode_count()) is subject to failures so the callback has > to be able to report errors. And, that itself is problematic because > we can't afford to have errors in dquot_free_inode(). If you have > thoughts about how to address this please let me know. Well, you can just make dquot_free_inode() return error. Now most callers won't be able to do much with an error from dquot_free_inode() but that's the case also for other things during inode deletion - just handle it as other fatal failures during inode freeing. > Alternatively, I could try to make this patch less intrusive by > keeping the existing dquot_transfer() signature and add a new > dquot_transfer_usage() that accepts inode_usage as a parameter. What > do you think? That would be somewhat better than what you do in this patch but I prefer to handle this like I suggested above. Honza
>> I tried that approach by adding a "int get_inode_usage(struct inode >> *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, >> ext4 code that calculates the number of internal inodes >> (ext4_xattr_inode_count()) is subject to failures so the callback has >> to be able to report errors. And, that itself is problematic because >> we can't afford to have errors in dquot_free_inode(). If you have >> thoughts about how to address this please let me know. > > Well, you can just make dquot_free_inode() return error. Now most callers > won't be able to do much with an error from dquot_free_inode() but that's > the case also for other things during inode deletion - just handle it as > other fatal failures during inode freeing. > I just checked dquot_free_inode() to see whether it calls anything that could fail. It calls mark_all_dquot_dirty() and ignores the return code from it. I would like to follow the same for the get_inode_usage() as the only use case for get_inode_usage() (ext4) should not fail at inode free time. Basically, I want to avoid changing return type from void to int because it would create a new responsibility for the filesystem implementations who do not know how to deal with it.
On Mon 19-06-17 04:46:00, Tahsin Erdogan wrote: > >> I tried that approach by adding a "int get_inode_usage(struct inode > >> *inode, qsize_t *usage)" callback to dquot_operations. Unfortunately, > >> ext4 code that calculates the number of internal inodes > >> (ext4_xattr_inode_count()) is subject to failures so the callback has > >> to be able to report errors. And, that itself is problematic because > >> we can't afford to have errors in dquot_free_inode(). If you have > >> thoughts about how to address this please let me know. > > > > Well, you can just make dquot_free_inode() return error. Now most callers > > won't be able to do much with an error from dquot_free_inode() but that's > > the case also for other things during inode deletion - just handle it as > > other fatal failures during inode freeing. > > > I just checked dquot_free_inode() to see whether it calls anything > that could fail. It calls mark_all_dquot_dirty() and ignores the > return code from it. I would like to follow the same for the > get_inode_usage() as the only use case for get_inode_usage() (ext4) > should not fail at inode free time. > > Basically, I want to avoid changing return type from void to int > because it would create a new responsibility for the filesystem > implementations who do not know how to deal with it. Heh, this "pushing of responsibility" looks like a silly game. If an error can happen in a function, it is better to report it as far as easily possible (unless we can cleanly handle it which we cannot here). I'm guilty of making dquot_free_inode() ignore errors from mark_all_dquot_dirty() and in retrospect it would have been better if these were propagated to the caller as well. And eventually we can fix this if we decide we care enough. I'm completely fine with just returning an error from dquot_free_inode() and ignore it in all the callers except for ext4. Then filesystems which care enough can try to handle the error. That way we at least don't increase the design debt from the past. Honza
On Mon, Jun 19, 2017 at 5:36 AM, Jan Kara <jack@suse.cz> wrote: > Heh, this "pushing of responsibility" looks like a silly game. If an error > can happen in a function, it is better to report it as far as easily > possible (unless we can cleanly handle it which we cannot here). I'm guilty > of making dquot_free_inode() ignore errors from mark_all_dquot_dirty() and > in retrospect it would have been better if these were propagated to the > caller as well. And eventually we can fix this if we decide we care enough. > I'm completely fine with just returning an error from dquot_free_inode() > and ignore it in all the callers except for ext4. Then filesystems which > care enough can try to handle the error. That way we at least don't > increase the design debt from the past. I sent an update but since patch title changed it landed in a new email thread I think ("[PATCH v2 28/31] quota: add get_inode_usage callback to transfer multi-inode charges"). I will respond to your comment in that thread.
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 2dcbd5698884..a13ba5dcb355 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -1656,7 +1656,7 @@ int ext2_setattr(struct dentry *dentry, struct iattr *iattr) } if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) { - error = dquot_transfer(inode, iattr); + error = dquot_transfer(inode, iattr, 0); if (error) return error; } diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 6f5872197d6c..28abbbdbbb80 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5267,6 +5267,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) int error, rc = 0; int orphan = 0; const unsigned int ia_valid = attr->ia_valid; + int ea_inode_refs; if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) return -EIO; @@ -5293,7 +5294,12 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) error = PTR_ERR(handle); goto err_out; } - error = dquot_transfer(inode, attr); + + down_read(&EXT4_I(inode)->xattr_sem); + error = ea_inode_refs = ext4_xattr_inode_count(inode); + if (ea_inode_refs >= 0) + error = dquot_transfer(inode, attr, ea_inode_refs); + 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..9938dc8e24c8 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -21,6 +21,7 @@ #include "ext4.h" #include <linux/fsmap.h> #include "fsmap.h" +#include "xattr.h" #include <trace/events/ext4.h> /** @@ -319,6 +320,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) struct ext4_iloc iloc; struct ext4_inode *raw_inode; struct dquot *transfer_to[MAXQUOTAS] = { }; + int ea_inode_refs; if (!ext4_has_feature_project(sb)) { if (projid != EXT4_DEF_PROJID) @@ -371,9 +373,17 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) if (err) goto out_stop; + down_read(&EXT4_I(inode)->xattr_sem); + ea_inode_refs = ext4_xattr_inode_count(inode); + if (ea_inode_refs < 0) { + up_read(&EXT4_I(inode)->xattr_sem); + err = ea_inode_refs; + goto out_stop; + } + transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid)); if (!IS_ERR(transfer_to[PRJQUOTA])) { - err = __dquot_transfer(inode, transfer_to); + err = __dquot_transfer(inode, transfer_to, ea_inode_refs); dqput(transfer_to[PRJQUOTA]); if (err) goto out_dirty; @@ -382,6 +392,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid) EXT4_I(inode)->i_projid = kprojid; inode->i_ctime = current_time(inode); out_dirty: + up_read(&EXT4_I(inode)->xattr_sem); rc = ext4_mark_iloc_dirty(handle, inode, &iloc); if (!err) err = rc; diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c index caddc176a612..1d6fcbb01517 100644 --- a/fs/ext4/xattr.c +++ b/fs/ext4/xattr.c @@ -701,6 +701,60 @@ static void ext4_xattr_update_super_block(handle_t *handle, } } +int ext4_xattr_inode_count(struct inode *inode) +{ + 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; + int inode_count = 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) + inode_count++; + } + + 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) + inode_count++; + } + ret = inode_count; +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..8ef6fe123255 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 + +int ext4_xattr_inode_count(struct inode *inode); diff --git a/fs/jfs/file.c b/fs/jfs/file.c index 739492c7a3fd..b08e0b0449a7 100644 --- a/fs/jfs/file.c +++ b/fs/jfs/file.c @@ -114,7 +114,7 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr) } if ((iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)) || (iattr->ia_valid & ATTR_GID && !gid_eq(iattr->ia_gid, inode->i_gid))) { - rc = dquot_transfer(inode, iattr); + rc = dquot_transfer(inode, iattr, 0); if (rc) return rc; } diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index bfeb647459d9..d3cbf6467af6 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -1259,7 +1259,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) mlog_errno(status); goto bail_unlock; } - status = __dquot_transfer(inode, transfer_to); + status = __dquot_transfer(inode, transfer_to, 0); if (status < 0) goto bail_commit; } else { diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 48813aeaab80..16e13d554aaa 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -1906,10 +1906,12 @@ EXPORT_SYMBOL(dquot_free_inode); * We are holding reference on transfer_from & transfer_to, no need to * protect them by srcu_read_lock(). */ -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to) +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to, + int inodes_extra) { qsize_t space, cur_space; qsize_t rsv_space = 0; + qsize_t inode_count = 1 + inodes_extra; struct dquot *transfer_from[MAXQUOTAS] = {}; int cnt, ret = 0; char is_valid[MAXQUOTAS] = {}; @@ -1946,7 +1948,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_count, &warn_to[cnt]); if (ret) goto over_quota; ret = check_bdq(transfer_to[cnt], space, 0, &warn_to[cnt]); @@ -1963,7 +1965,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_count); if (wtype != QUOTA_NL_NOWARN) prepare_warning(&warn_from_inodes[cnt], transfer_from[cnt], wtype); @@ -1971,13 +1973,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_count); 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_count); dquot_incr_space(transfer_to[cnt], cur_space); dquot_resv_space(transfer_to[cnt], rsv_space); @@ -2005,7 +2007,7 @@ EXPORT_SYMBOL(__dquot_transfer); /* Wrapper for transferring ownership of an inode for uid/gid only * Called from FSXXX_setattr() */ -int dquot_transfer(struct inode *inode, struct iattr *iattr) +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra) { struct dquot *transfer_to[MAXQUOTAS] = {}; struct dquot *dquot; @@ -2037,7 +2039,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr) } transfer_to[GRPQUOTA] = dquot; } - ret = __dquot_transfer(inode, transfer_to); + ret = __dquot_transfer(inode, transfer_to, inodes_extra); out_put: dqput_all(transfer_to); return ret; diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c index 873fc04e9403..51586051b5dd 100644 --- a/fs/reiserfs/inode.c +++ b/fs/reiserfs/inode.c @@ -3370,7 +3370,7 @@ int reiserfs_setattr(struct dentry *dentry, struct iattr *attr) reiserfs_write_unlock(inode->i_sb); if (error) goto out; - error = dquot_transfer(inode, attr); + error = dquot_transfer(inode, attr, 0); reiserfs_write_lock(inode->i_sb); if (error) { journal_end(&th); diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index dda22f45fc1b..b7bcd9c6db6c 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -106,8 +106,9 @@ int dquot_get_next_dqblk(struct super_block *sb, struct kqid *id, int dquot_set_dqblk(struct super_block *sb, struct kqid id, struct qc_dqblk *di); -int __dquot_transfer(struct inode *inode, struct dquot **transfer_to); -int dquot_transfer(struct inode *inode, struct iattr *iattr); +int __dquot_transfer(struct inode *inode, struct dquot **transfer_to, + int inodes_extra); +int dquot_transfer(struct inode *inode, struct iattr *iattr, int inodes_extra); static inline struct mem_dqinfo *sb_dqinfo(struct super_block *sb, int type) { @@ -226,7 +227,8 @@ static inline void dquot_free_inode(struct inode *inode) { } -static inline int dquot_transfer(struct inode *inode, struct iattr *iattr) +static inline int dquot_transfer(struct inode *inode, struct iattr *iattr, + int inodes_extra) { return 0; }
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. In our case, we would like to specify additional inodes to be transferred. Signed-off-by: Tahsin Erdogan <tahsin@google.com> --- fs/ext2/inode.c | 2 +- fs/ext4/inode.c | 8 ++++++- fs/ext4/ioctl.c | 13 +++++++++++- fs/ext4/xattr.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ fs/ext4/xattr.h | 2 ++ fs/jfs/file.c | 2 +- fs/ocfs2/file.c | 2 +- fs/quota/dquot.c | 16 +++++++------- fs/reiserfs/inode.c | 2 +- include/linux/quotaops.h | 8 ++++--- 10 files changed, 93 insertions(+), 16 deletions(-)