ext4: xattr-in-inode support

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().

Quoted 3 lines · Jan Kara

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?

Quoted 7 lines

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.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 19 lines · Tahsin Erdogan

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 3 lines

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.

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 <[email protected]>

---
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(-)

fs/ext4/inode.c +7 -0

index ea95bd9eab81..cd22de0b5d2c 100644
@@ -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;

fs/ext4/ioctl.c +6 -0

index dde8deb11e59..42b3a73143cf 100644
@@ -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;

fs/ext4/super.c +11 -10

index 2bfacd737bb6..4b15bf674d45 100644
@@ -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 = {

fs/ext4/xattr.c +54 -0

index d7e60358ec91..5e20f29afe9e 100644
@@ -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;

fs/ext4/xattr.h +2 -0

index 67616cb9a059..26119a67c8c3 100644
@@ -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);

fs/quota/dquot.c +12 -4

index 48813aeaab80..53a17496c5c5 100644
@@ -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);
 

include/linux/quota.h +2 -1

index 3434eef2a5aa..bfd077ca6ac3 100644
@@ -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

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 <[email protected]>

---
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(-)

fs/ext4/inode.c +7 -0

index ea95bd9eab81..cd22de0b5d2c 100644
@@ -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;

fs/ext4/ioctl.c +6 -0

index dde8deb11e59..42b3a73143cf 100644
@@ -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;

fs/ext4/super.c +11 -10

index 2bfacd737bb6..4b15bf674d45 100644
@@ -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 = {

fs/ext4/xattr.c +54 -0

index d7e60358ec91..5e20f29afe9e 100644
@@ -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;

fs/ext4/xattr.h +2 -0

index 67616cb9a059..26119a67c8c3 100644
@@ -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);

fs/quota/dquot.c +12 -4

index 48813aeaab80..53a17496c5c5 100644
@@ -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);
 

include/linux/quota.h +2 -0

index 3434eef2a5aa..bfd077ca6ac3 100644
@@ -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);
 };

Signature · 1 line

2.13.1.518.g3df882009-goog

Tahsin, when you think we've closed on the reviews, could you send out a complete set of all of the patches on a new mail thread, using git send-email so I can make sure I'm grabbing the final version of all of the patches in this patch series?

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.

From: Andreas Dilger <[email protected]>

Large xattr support is implemented for EXT4_FEATURE_INCOMPAT_EA_INODE.

If the size of an xattr value is larger than will fit in a single external block, then the xattr value will be saved into the body of an external xattr inode.

The also helps support a larger number of xattr, since only the headers will be stored in the in-inode space or the single external block.

The inode is referenced from the xattr header via "e_value_inum", which was formerly "e_value_block", but that field was never used. The e_value_size still contains the xattr size so that listing xattrs does not need to look up the inode if the data is not accessed.

struct ext4_xattr_entry {

        __u8    e_name_len;     /* length of name */
        __u8    e_name_index;   /* attribute name index */
        __le16  e_value_offs;   /* offset in disk block of value */
        __le32  e_value_inum;   /* inode in which value is stored */
        __le32  e_value_size;   /* size of attribute value */
        __le32  e_hash;         /* hash value of name and value */
        char    e_name[0];      /* attribute name */

};

The xattr inode is marked with the EXT4_EA_INODE_FL flag and also holds a back-reference to the owning inode in its i_mtime field, allowing the ext4/e2fsck to verify the correct inode is accessed.

Lustre-Jira: https://jira.hpdd.intel.com/browse/LU-80
Lustre-bugzilla: https://bugzilla.lustre.org/show_bug.cgi?id=4424

Signed-off-by Kalpak Shah <[email protected]>

Signed-off-by James Simmons <[email protected]>

Signed-off-by Andreas Dilger <[email protected]>

Signed-off-by Tahsin Erdogan <[email protected]>

---
 fs/ext4/ext4.h   |  12 ++
 fs/ext4/ialloc.c |   1 -
 fs/ext4/inline.c |   2 +-
 fs/ext4/inode.c  |  49 ++++-
 fs/ext4/xattr.c  | 565 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/ext4/xattr.h  |  33 +++-
 6 files changed, 606 insertions(+), 56 deletions(-)

fs/ext4/ext4.h +12 -0

index 32191548abed..24ef56b4572f 100644
@@ -1797,6 +1797,7 @@ EXT4_FEATURE_INCOMPAT_FUNCS(encrypt,		ENCRYPT)
 					 EXT4_FEATURE_INCOMPAT_EXTENTS| \
 					 EXT4_FEATURE_INCOMPAT_64BIT| \
 					 EXT4_FEATURE_INCOMPAT_FLEX_BG| \
+					 EXT4_FEATURE_INCOMPAT_EA_INODE| \
 					 EXT4_FEATURE_INCOMPAT_MMP | \
 					 EXT4_FEATURE_INCOMPAT_INLINE_DATA | \
 					 EXT4_FEATURE_INCOMPAT_ENCRYPT | \

@@ -2220,6 +2221,12 @@ struct mmpd_data {
 #define EXT4_MMP_MAX_CHECK_INTERVAL	300UL
 
 /*
+ * Maximum size of xattr attributes for FEATURE_INCOMPAT_EA_INODE 1Mb
+ * This limit is arbitrary, but is reasonable for the xattr API.
+ */
+#define EXT4_XATTR_MAX_LARGE_EA_SIZE    (1024 * 1024)
+
+/*
  * Function prototypes
  */
 

@@ -2231,6 +2238,10 @@ struct mmpd_data {
 # define ATTRIB_NORET	__attribute__((noreturn))
 # define NORET_AND	noreturn,
 
+struct ext4_xattr_ino_array {
+	unsigned int xia_count;		/* # of used item in the array */
+	unsigned int xia_inodes[0];
+};
 /* bitmap.c */
 extern unsigned int ext4_count_free(char *bitmap, unsigned numchars);
 void ext4_inode_bitmap_csum_set(struct super_block *sb, ext4_group_t group,

@@ -2478,6 +2489,7 @@ extern int ext4_truncate_restart_trans(handle_t *, struct inode *, int nblocks);
 extern void ext4_set_inode_flags(struct inode *);
 extern int ext4_alloc_da_blocks(struct inode *inode);
 extern void ext4_set_aops(struct inode *inode);
+extern int ext4_meta_trans_blocks(struct inode *, int nrblocks, int chunk);
 extern int ext4_writepage_trans_blocks(struct inode *);
 extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
 extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,

fs/ext4/ialloc.c +0 -1

index 98ac2f1f23b3..e2eb3cc06820 100644
@@ -294,7 +294,6 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
 	 * as writing the quota to disk may need the lock as well.
 	 */
 	dquot_initialize(inode);
-	ext4_xattr_delete_inode(handle, inode);
 	dquot_free_inode(inode);
 	dquot_drop(inode);
 

fs/ext4/inline.c +1 -1

index 8d141c0c8ff9..28c5c3abddb3 100644
@@ -61,7 +61,7 @@ static int get_max_inline_xattr_value_size(struct inode *inode,
 
 	/* Compute min_offs. */
 	for (; !IS_LAST_ENTRY(entry); entry = EXT4_XATTR_NEXT(entry)) {
-		if (!entry->e_value_block && entry->e_value_size) {
+		if (!entry->e_value_inum && entry->e_value_size) {
 			size_t offs = le16_to_cpu(entry->e_value_offs);
 			if (offs < min_offs)
 				min_offs = offs;

fs/ext4/inode.c +40 -9

index 5cf82d03968c..e5535e5b3dc5 100644
@@ -139,8 +139,6 @@ static void ext4_invalidatepage(struct page *page, unsigned int offset,
 				unsigned int length);
 static int __ext4_journalled_writepage(struct page *page, unsigned int len);
 static int ext4_bh_delay_or_unwritten(handle_t *handle, struct buffer_head *bh);
-static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
-				  int pextents);
 
 /*
  * Test whether an inode is a fast symlink.

@@ -189,6 +187,8 @@ void ext4_evict_inode(struct inode *inode)
 {
 	handle_t *handle;
 	int err;
+	int extra_credits = 3;
+	struct ext4_xattr_ino_array *lea_ino_array = NULL;
 
 	trace_ext4_evict_inode(inode);
 

@@ -238,8 +238,8 @@ void ext4_evict_inode(struct inode *inode)
 	 * protection against it
 	 */
 	sb_start_intwrite(inode->i_sb);
-	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE,
-				    ext4_blocks_for_truncate(inode)+3);
+
+	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, extra_credits);
 	if (IS_ERR(handle)) {
 		ext4_std_error(inode->i_sb, PTR_ERR(handle));
 		/*

@@ -251,9 +251,36 @@ void ext4_evict_inode(struct inode *inode)
 		sb_end_intwrite(inode->i_sb);
 		goto no_delete;
 	}
-
 	if (IS_SYNC(inode))
 		ext4_handle_sync(handle);
+
+	/*
+	 * Delete xattr inode before deleting the main inode.
+	 */
+	err = ext4_xattr_delete_inode(handle, inode, &lea_ino_array);
+	if (err) {
+		ext4_warning(inode->i_sb,
+			     "couldn't delete inode's xattr (err %d)", err);
+		goto stop_handle;
+	}
+
+	if (!IS_NOQUOTA(inode))
+		extra_credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb);
+
+	if (!ext4_handle_has_enough_credits(handle,
+			ext4_blocks_for_truncate(inode) + extra_credits)) {
+		err = ext4_journal_extend(handle,
+			ext4_blocks_for_truncate(inode) + extra_credits);
+		if (err > 0)
+			err = ext4_journal_restart(handle,
+			ext4_blocks_for_truncate(inode) + extra_credits);
+		if (err != 0) {
+			ext4_warning(inode->i_sb,
+				     "couldn't extend journal (err %d)", err);
+			goto stop_handle;
+		}
+	}
+
 	inode->i_size = 0;
 	err = ext4_mark_inode_dirty(handle, inode);
 	if (err) {

@@ -277,10 +304,10 @@ void ext4_evict_inode(struct inode *inode)
 	 * enough credits left in the handle to remove the inode from
 	 * the orphan list and set the dtime field.
 	 */
-	if (!ext4_handle_has_enough_credits(handle, 3)) {
-		err = ext4_journal_extend(handle, 3);
+	if (!ext4_handle_has_enough_credits(handle, extra_credits)) {
+		err = ext4_journal_extend(handle, extra_credits);
 		if (err > 0)
-			err = ext4_journal_restart(handle, 3);
+			err = ext4_journal_restart(handle, extra_credits);
 		if (err != 0) {
 			ext4_warning(inode->i_sb,
 				     "couldn't extend journal (err %d)", err);

@@ -315,8 +342,12 @@ void ext4_evict_inode(struct inode *inode)
 		ext4_clear_inode(inode);
 	else
 		ext4_free_inode(handle, inode);
+
 	ext4_journal_stop(handle);
 	sb_end_intwrite(inode->i_sb);
+
+	if (lea_ino_array != NULL)
+		ext4_xattr_inode_array_free(inode, lea_ino_array);
 	return;
 no_delete:
 	ext4_clear_inode(inode);	/* We must guarantee clearing of inode... */

@@ -5504,7 +5535,7 @@ static int ext4_index_trans_blocks(struct inode *inode, int lblocks,
  *
  * Also account for superblock, inode, quota and xattr blocks
  */
-static int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
+int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
 				  int pextents)
 {
 	ext4_group_t groups, ngroups = ext4_get_groups_count(inode->i_sb);

fs/ext4/xattr.c +523 -42

index 5d3c2536641c..444be5c7a1d5 100644
@@ -177,9 +177,8 @@ ext4_xattr_check_entries(struct ext4_xattr_entry *entry, void *end,
 
 	/* Check the values */
 	while (!IS_LAST_ENTRY(entry)) {
-		if (entry->e_value_block != 0)
-			return -EFSCORRUPTED;
-		if (entry->e_value_size != 0) {
+		if (entry->e_value_size != 0 &&
+		    entry->e_value_inum == 0) {
 			u16 offs = le16_to_cpu(entry->e_value_offs);
 			u32 size = le32_to_cpu(entry->e_value_size);
 			void *value;

@@ -269,6 +268,99 @@ ext4_xattr_find_entry(struct ext4_xattr_entry **pentry, int name_index,
 	return cmp ? -ENODATA : 0;
 }
 
+/*
+ * Read the EA value from an inode.
+ */
+static int
+ext4_xattr_inode_read(struct inode *ea_inode, void *buf, size_t *size)
+{
+	unsigned long block = 0;
+	struct buffer_head *bh = NULL;
+	int blocksize;
+	size_t csize, ret_size = 0;
+
+	if (*size == 0)
+		return 0;
+
+	blocksize = ea_inode->i_sb->s_blocksize;
+
+	while (ret_size < *size) {
+		csize = (*size - ret_size) > blocksize ? blocksize :
+							*size - ret_size;
+		bh = ext4_bread(NULL, ea_inode, block, 0);
+		if (IS_ERR(bh)) {
+			*size = ret_size;
+			return PTR_ERR(bh);
+		}
+		memcpy(buf, bh->b_data, csize);
+		brelse(bh);
+
+		buf += csize;
+		block += 1;
+		ret_size += csize;
+	}
+
+	*size = ret_size;
+
+	return 0;
+}
+
+struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino, int *err)
+{
+	struct inode *ea_inode = NULL;
+
+	ea_inode = ext4_iget(parent->i_sb, ea_ino);
+	if (IS_ERR(ea_inode) || is_bad_inode(ea_inode)) {
+		int rc = IS_ERR(ea_inode) ? PTR_ERR(ea_inode) : 0;
+		ext4_error(parent->i_sb, "error while reading EA inode %lu "
+			   "/ %d %d", ea_ino, rc, is_bad_inode(ea_inode));
+		*err = rc != 0 ? rc : -EIO;
+		return NULL;
+	}
+
+	if (EXT4_XATTR_INODE_GET_PARENT(ea_inode) != parent->i_ino ||
+	    ea_inode->i_generation != parent->i_generation) {
+		ext4_error(parent->i_sb, "Backpointer from EA inode %lu "
+			   "to parent invalid.", ea_ino);
+		*err = -EINVAL;
+		goto error;
+	}
+
+	if (!(EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL)) {
+		ext4_error(parent->i_sb, "EA inode %lu does not have "
+			   "EXT4_EA_INODE_FL flag set.\n", ea_ino);
+		*err = -EINVAL;
+		goto error;
+	}
+
+	*err = 0;
+	return ea_inode;
+
+error:
+	iput(ea_inode);
+	return NULL;
+}
+
+/*
+ * Read the value from the EA inode.
+ */
+static int
+ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
+		     size_t *size)
+{
+	struct inode *ea_inode = NULL;
+	int err;
+
+	ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
+	if (err)
+		return err;
+
+	err = ext4_xattr_inode_read(ea_inode, buffer, size);
+	iput(ea_inode);
+
+	return err;
+}
+
 static int
 ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 		     void *buffer, size_t buffer_size)

@@ -308,8 +400,16 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 		error = -ERANGE;
 		if (size > buffer_size)
 			goto cleanup;
-		memcpy(buffer, bh->b_data + le16_to_cpu(entry->e_value_offs),
-		       size);
+		if (entry->e_value_inum) {
+			error = ext4_xattr_inode_get(inode,
+					     le32_to_cpu(entry->e_value_inum),
+					     buffer, &size);
+			if (error)
+				goto cleanup;
+		} else {
+			memcpy(buffer, bh->b_data +
+			       le16_to_cpu(entry->e_value_offs), size);
+		}
 	}
 	error = size;
 

@@ -350,8 +450,16 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 		error = -ERANGE;
 		if (size > buffer_size)
 			goto cleanup;
-		memcpy(buffer, (void *)IFIRST(header) +
-		       le16_to_cpu(entry->e_value_offs), size);
+		if (entry->e_value_inum) {
+			error = ext4_xattr_inode_get(inode,
+					     le32_to_cpu(entry->e_value_inum),
+					     buffer, &size);
+			if (error)
+				goto cleanup;
+		} else {
+			memcpy(buffer, (void *)IFIRST(header) +
+			       le16_to_cpu(entry->e_value_offs), size);
+		}
 	}
 	error = size;
 

@@ -620,7 +728,7 @@ static size_t ext4_xattr_free_space(struct ext4_xattr_entry *last,
 				    size_t *min_offs, void *base, int *total)
 {
 	for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
-		if (last->e_value_size) {
+		if (!last->e_value_inum && last->e_value_size) {
 			size_t offs = le16_to_cpu(last->e_value_offs);
 			if (offs < *min_offs)
 				*min_offs = offs;

@@ -631,16 +739,173 @@ static size_t ext4_xattr_free_space(struct ext4_xattr_entry *last,
 	return (*min_offs - ((void *)last - base) - sizeof(__u32));
 }
 
-static int
-ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
+/*
+ * Write the value of the EA in an inode.
+ */
+static int ext4_xattr_inode_write(handle_t *handle, struct inode *ea_inode,
+				  const void *buf, int bufsize)
+{
+	struct buffer_head *bh = NULL;
+	unsigned long block = 0;
+	unsigned blocksize = ea_inode->i_sb->s_blocksize;
+	unsigned max_blocks = (bufsize + blocksize - 1) >> ea_inode->i_blkbits;
+	int csize, wsize = 0;
+	int ret = 0;
+	int retries = 0;
+
+retry:
+	while (ret >= 0 && ret < max_blocks) {
+		struct ext4_map_blocks map;
+		map.m_lblk = block += ret;
+		map.m_len = max_blocks -= ret;
+
+		ret = ext4_map_blocks(handle, ea_inode, &map,
+				      EXT4_GET_BLOCKS_CREATE);
+		if (ret <= 0) {
+			ext4_mark_inode_dirty(handle, ea_inode);
+			if (ret == -ENOSPC &&
+			    ext4_should_retry_alloc(ea_inode->i_sb, &retries)) {
+				ret = 0;
+				goto retry;
+			}
+			break;
+		}
+	}
+
+	if (ret < 0)
+		return ret;
+
+	block = 0;
+	while (wsize < bufsize) {
+		if (bh != NULL)
+			brelse(bh);
+		csize = (bufsize - wsize) > blocksize ? blocksize :
+								bufsize - wsize;
+		bh = ext4_getblk(handle, ea_inode, block, 0);
+		if (IS_ERR(bh)) {
+			ret = PTR_ERR(bh);
+			goto out;
+		}
+		ret = ext4_journal_get_write_access(handle, bh);
+		if (ret)
+			goto out;
+
+		memcpy(bh->b_data, buf, csize);
+		set_buffer_uptodate(bh);
+		ext4_handle_dirty_metadata(handle, ea_inode, bh);
+
+		buf += csize;
+		wsize += csize;
+		block += 1;
+	}
+
+	inode_lock(ea_inode);
+	i_size_write(ea_inode, wsize);
+	ext4_update_i_disksize(ea_inode, wsize);
+	inode_unlock(ea_inode);
+
+	ext4_mark_inode_dirty(handle, ea_inode);
+
+out:
+	brelse(bh);
+
+	return ret;
+}
+
+/*
+ * Create an inode to store the value of a large EA.
+ */
+static struct inode *ext4_xattr_inode_create(handle_t *handle,
+					     struct inode *inode)
+{
+	struct inode *ea_inode = NULL;
+
+	/*
+	 * Let the next inode be the goal, so we try and allocate the EA inode
+	 * in the same group, or nearby one.
+	 */
+	ea_inode = ext4_new_inode(handle, inode->i_sb->s_root->d_inode,
+				  S_IFREG | 0600, NULL, inode->i_ino + 1, NULL);
+	if (!IS_ERR(ea_inode)) {
+		ea_inode->i_op = &ext4_file_inode_operations;
+		ea_inode->i_fop = &ext4_file_operations;
+		ext4_set_aops(ea_inode);
+		ea_inode->i_generation = inode->i_generation;
+		EXT4_I(ea_inode)->i_flags |= EXT4_EA_INODE_FL;
+
+		/*
+		 * A back-pointer from EA inode to parent inode will be useful
+		 * for e2fsck.
+		 */
+		EXT4_XATTR_INODE_SET_PARENT(ea_inode, inode->i_ino);
+		unlock_new_inode(ea_inode);
+	}
+
+	return ea_inode;
+}
+
+/*
+ * Unlink the inode storing the value of the EA.
+ */
+int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino)
+{
+	struct inode *ea_inode = NULL;
+	int err;
+
+	ea_inode = ext4_xattr_inode_iget(inode, ea_ino, &err);
+	if (err)
+		return err;
+
+	clear_nlink(ea_inode);
+	iput(ea_inode);
+
+	return 0;
+}
+
+/*
+ * Add value of the EA in an inode.
+ */
+static int ext4_xattr_inode_set(handle_t *handle, struct inode *inode,
+				unsigned long *ea_ino, const void *value,
+				size_t value_len)
+{
+	struct inode *ea_inode;
+	int err;
+
+	/* Create an inode for the EA value */
+	ea_inode = ext4_xattr_inode_create(handle, inode);
+	if (IS_ERR(ea_inode))
+		return PTR_ERR(ea_inode);
+
+	err = ext4_xattr_inode_write(handle, ea_inode, value, value_len);
+	if (err)
+		clear_nlink(ea_inode);
+	else
+		*ea_ino = ea_inode->i_ino;
+
+	iput(ea_inode);
+
+	return err;
+}
+
+static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
+				struct ext4_xattr_search *s,
+				handle_t *handle, struct inode *inode)
 {
 	struct ext4_xattr_entry *last;
 	size_t free, min_offs = s->end - s->base, name_len = strlen(i->name);
+	int in_inode = i->in_inode;
+	int rc;
+
+	if (ext4_has_feature_ea_inode(inode->i_sb) &&
+	    (EXT4_XATTR_SIZE(i->value_len) >
+	     EXT4_XATTR_MIN_LARGE_EA_SIZE(inode->i_sb->s_blocksize)))
+		in_inode = 1;
 
 	/* Compute min_offs and last. */
 	last = s->first;
 	for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
-		if (last->e_value_size) {
+		if (!last->e_value_inum && last->e_value_size) {
 			size_t offs = le16_to_cpu(last->e_value_offs);
 			if (offs < min_offs)
 				min_offs = offs;

@@ -648,15 +913,20 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
 	}
 	free = min_offs - ((void *)last - s->base) - sizeof(__u32);
 	if (!s->not_found) {
-		if (s->here->e_value_size) {
+		if (!in_inode &&
+		    !s->here->e_value_inum && s->here->e_value_size) {
 			size_t size = le32_to_cpu(s->here->e_value_size);
 			free += EXT4_XATTR_SIZE(size);
 		}
 		free += EXT4_XATTR_LEN(name_len);
 	}
 	if (i->value) {
-		if (free < EXT4_XATTR_LEN(name_len) +
-			   EXT4_XATTR_SIZE(i->value_len))
+		size_t value_len = EXT4_XATTR_SIZE(i->value_len);
+
+		if (in_inode)
+			value_len = 0;
+
+		if (free < EXT4_XATTR_LEN(name_len) + value_len)
 			return -ENOSPC;
 	}
 

@@ -670,7 +940,8 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
 		s->here->e_name_len = name_len;
 		memcpy(s->here->e_name, i->name, name_len);
 	} else {
-		if (s->here->e_value_size) {
+		if (!s->here->e_value_inum && s->here->e_value_size &&
+		    s->here->e_value_offs > 0) {
 			void *first_val = s->base + min_offs;
 			size_t offs = le16_to_cpu(s->here->e_value_offs);
 			void *val = s->base + offs;

@@ -704,12 +975,18 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
 			last = s->first;
 			while (!IS_LAST_ENTRY(last)) {
 				size_t o = le16_to_cpu(last->e_value_offs);
-				if (last->e_value_size && o < offs)
+				if (!last->e_value_inum &&
+				    last->e_value_size && o < offs)
 					last->e_value_offs =
 						cpu_to_le16(o + size);
 				last = EXT4_XATTR_NEXT(last);
 			}
 		}
+		if (s->here->e_value_inum) {
+			ext4_xattr_inode_unlink(inode,
+					    le32_to_cpu(s->here->e_value_inum));
+			s->here->e_value_inum = 0;
+		}
 		if (!i->value) {
 			/* Remove the old name. */
 			size_t size = EXT4_XATTR_LEN(name_len);

@@ -722,11 +999,20 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
 
 	if (i->value) {
 		/* Insert the new value. */
-		s->here->e_value_size = cpu_to_le32(i->value_len);
-		if (i->value_len) {
+		if (in_inode) {
+			unsigned long ea_ino =
+				le32_to_cpu(s->here->e_value_inum);
+			rc = ext4_xattr_inode_set(handle, inode, &ea_ino,
+						  i->value, i->value_len);
+			if (rc)
+				goto out;
+			s->here->e_value_inum = cpu_to_le32(ea_ino);
+			s->here->e_value_offs = 0;
+		} else if (i->value_len) {
 			size_t size = EXT4_XATTR_SIZE(i->value_len);
 			void *val = s->base + min_offs - size;
 			s->here->e_value_offs = cpu_to_le16(min_offs - size);
+			s->here->e_value_inum = 0;
 			if (i->value == EXT4_ZERO_XATTR_VALUE) {
 				memset(val, 0, size);
 			} else {

@@ -736,8 +1022,11 @@ ext4_xattr_set_entry(struct ext4_xattr_info *i, struct ext4_xattr_search *s)
 				memcpy(val, i->value, i->value_len);
 			}
 		}
+		s->here->e_value_size = cpu_to_le32(i->value_len);
 	}
-	return 0;
+
+out:
+	return rc;
 }
 
 struct ext4_xattr_block_find {

@@ -801,8 +1090,6 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 
 #define header(x) ((struct ext4_xattr_header *)(x))
 
-	if (i->value && i->value_len > sb->s_blocksize)
-		return -ENOSPC;
 	if (s->base) {
 		BUFFER_TRACE(bs->bh, "get_write_access");
 		error = ext4_journal_get_write_access(handle, bs->bh);

@@ -821,7 +1108,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			mb_cache_entry_delete_block(ext4_mb_cache, hash,
 						    bs->bh->b_blocknr);
 			ea_bdebug(bs->bh, "modifying in-place");
-			error = ext4_xattr_set_entry(i, s);
+			error = ext4_xattr_set_entry(i, s, handle, inode);
 			if (!error) {
 				if (!IS_LAST_ENTRY(s->first))
 					ext4_xattr_rehash(header(s->base),

@@ -870,7 +1157,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 		s->end = s->base + sb->s_blocksize;
 	}
 
-	error = ext4_xattr_set_entry(i, s);
+	error = ext4_xattr_set_entry(i, s, handle, inode);
 	if (error == -EFSCORRUPTED)
 		goto bad_block;
 	if (error)

@@ -1070,7 +1357,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 
 	if (EXT4_I(inode)->i_extra_isize == 0)
 		return -ENOSPC;
-	error = ext4_xattr_set_entry(i, s);
+	error = ext4_xattr_set_entry(i, s, handle, inode);
 	if (error) {
 		if (error == -ENOSPC &&
 		    ext4_has_inline_data(inode)) {

@@ -1082,7 +1369,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 			error = ext4_xattr_ibody_find(inode, i, is);
 			if (error)
 				return error;
-			error = ext4_xattr_set_entry(i, s);
+			error = ext4_xattr_set_entry(i, s, handle, inode);
 		}
 		if (error)
 			return error;

@@ -1098,7 +1385,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 	return 0;
 }
 
-static int ext4_xattr_ibody_set(struct inode *inode,
+static int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
 				struct ext4_xattr_info *i,
 				struct ext4_xattr_ibody_find *is)
 {

@@ -1108,7 +1395,7 @@ static int ext4_xattr_ibody_set(struct inode *inode,
 
 	if (EXT4_I(inode)->i_extra_isize == 0)
 		return -ENOSPC;
-	error = ext4_xattr_set_entry(i, s);
+	error = ext4_xattr_set_entry(i, s, handle, inode);
 	if (error)
 		return error;
 	header = IHDR(inode, ext4_raw_inode(&is->iloc));

@@ -1155,7 +1442,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 		.name = name,
 		.value = value,
 		.value_len = value_len,
-
+		.in_inode = 0,
 	};
 	struct ext4_xattr_ibody_find is = {
 		.s = { .not_found = -ENODATA, },

@@ -1204,7 +1491,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 	}
 	if (!value) {
 		if (!is.s.not_found)
-			error = ext4_xattr_ibody_set(inode, &i, &is);
+			error = ext4_xattr_ibody_set(handle, inode, &i, &is);
 		else if (!bs.s.not_found)
 			error = ext4_xattr_block_set(handle, inode, &i, &bs);
 	} else {

@@ -1215,7 +1502,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 		if (!bs.s.not_found && ext4_xattr_value_same(&bs.s, &i))
 			goto cleanup;
 
-		error = ext4_xattr_ibody_set(inode, &i, &is);
+		error = ext4_xattr_ibody_set(handle, inode, &i, &is);
 		if (!error && !bs.s.not_found) {
 			i.value = NULL;
 			error = ext4_xattr_block_set(handle, inode, &i, &bs);

@@ -1226,11 +1513,20 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 					goto cleanup;
 			}
 			error = ext4_xattr_block_set(handle, inode, &i, &bs);
+			if (ext4_has_feature_ea_inode(inode->i_sb) &&
+			    error == -ENOSPC) {
+				/* xattr not fit to block, store at external
+				 * inode */
+				i.in_inode = 1;
+				error = ext4_xattr_ibody_set(handle, inode,
+							     &i, &is);
+			}
 			if (error)
 				goto cleanup;
 			if (!is.s.not_found) {
 				i.value = NULL;
-				error = ext4_xattr_ibody_set(inode, &i, &is);
+				error = ext4_xattr_ibody_set(handle, inode, &i,
+							     &is);
 			}
 		}
 	}

@@ -1269,12 +1565,26 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name,
 	       const void *value, size_t value_len, int flags)
 {
 	handle_t *handle;
+	struct super_block *sb = inode->i_sb;
 	int error, retries = 0;
 	int credits = ext4_jbd2_credits_xattr(inode);
 
 	error = dquot_initialize(inode);
 	if (error)
 		return error;
+
+	if ((value_len >= EXT4_XATTR_MIN_LARGE_EA_SIZE(sb->s_blocksize)) &&
+	    ext4_has_feature_ea_inode(sb)) {
+		int nrblocks = (value_len + sb->s_blocksize - 1) >>
+					sb->s_blocksize_bits;
+
+		/* For new inode */
+		credits += EXT4_SINGLEDATA_TRANS_BLOCKS(sb) + 3;
+
+		/* For data blocks of EA inode */
+		credits += ext4_meta_trans_blocks(inode, nrblocks, 0);
+	}
+
 retry:
 	handle = ext4_journal_start(inode, EXT4_HT_XATTR, credits);
 	if (IS_ERR(handle)) {

@@ -1286,7 +1596,7 @@ ext4_xattr_set(struct inode *inode, int name_index, const char *name,
 					      value, value_len, flags);
 		error2 = ext4_journal_stop(handle);
 		if (error == -ENOSPC &&
-		    ext4_should_retry_alloc(inode->i_sb, &retries))
+		    ext4_should_retry_alloc(sb, &retries))
 			goto retry;
 		if (error == 0)
 			error = error2;

@@ -1311,7 +1621,7 @@ static void ext4_xattr_shift_entries(struct ext4_xattr_entry *entry,
 
 	/* Adjust the value offsets of the entries */
 	for (; !IS_LAST_ENTRY(last); last = EXT4_XATTR_NEXT(last)) {
-		if (last->e_value_size) {
+		if (!last->e_value_inum && last->e_value_size) {
 			new_offs = le16_to_cpu(last->e_value_offs) +
 							value_offs_shift;
 			last->e_value_offs = cpu_to_le16(new_offs);

@@ -1372,7 +1682,7 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
 		goto out;
 
 	/* Remove the chosen entry from the inode */
-	error = ext4_xattr_ibody_set(inode, &i, is);
+	error = ext4_xattr_ibody_set(handle, inode, &i, is);
 	if (error)
 		goto out;
 

@@ -1572,21 +1882,135 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
 }
 
 
+#define EIA_INCR 16 /* must be 2^n */
+#define EIA_MASK (EIA_INCR - 1)
+/* Add the large xattr @ino into @lea_ino_array for later deletion.
+ * If @lea_ino_array is new or full it will be grown and the old
+ * contents copied over.
+ */
+static int
+ext4_expand_ino_array(struct ext4_xattr_ino_array **lea_ino_array, __u32 ino)
+{
+	if (*lea_ino_array == NULL) {
+		/*
+		 * Start with 15 inodes, so it fits into a power-of-two size.
+		 * If *lea_ino_array is NULL, this is essentially offsetof()
+		 */
+		(*lea_ino_array) =
+			kmalloc(offsetof(struct ext4_xattr_ino_array,
+					 xia_inodes[EIA_MASK]),
+				GFP_NOFS);
+		if (*lea_ino_array == NULL)
+			return -ENOMEM;
+		(*lea_ino_array)->xia_count = 0;
+	} else if (((*lea_ino_array)->xia_count & EIA_MASK) == EIA_MASK) {
+		/* expand the array once all 15 + n * 16 slots are full */
+		struct ext4_xattr_ino_array *new_array = NULL;
+		int count = (*lea_ino_array)->xia_count;
+
+		/* if new_array is NULL, this is essentially offsetof() */
+		new_array = kmalloc(
+				offsetof(struct ext4_xattr_ino_array,
+					 xia_inodes[count + EIA_INCR]),
+				GFP_NOFS);
+		if (new_array == NULL)
+			return -ENOMEM;
+		memcpy(new_array, *lea_ino_array,
+		       offsetof(struct ext4_xattr_ino_array,
+				xia_inodes[count]));
+		kfree(*lea_ino_array);
+		*lea_ino_array = new_array;
+	}
+	(*lea_ino_array)->xia_inodes[(*lea_ino_array)->xia_count++] = ino;
+	return 0;
+}
+
+/**
+ * Add xattr inode to orphan list
+ */
+static int
+ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode,
+			int credits, struct ext4_xattr_ino_array *lea_ino_array)
+{
+	struct inode *ea_inode = NULL;
+	int idx = 0, error = 0;
+
+	if (lea_ino_array == NULL)
+		return 0;
+
+	for (; idx < lea_ino_array->xia_count; ++idx) {
+		if (!ext4_handle_has_enough_credits(handle, credits)) {
+			error = ext4_journal_extend(handle, credits);
+			if (error > 0)
+				error = ext4_journal_restart(handle, credits);
+
+			if (error != 0) {
+				ext4_warning(inode->i_sb,
+					"couldn't extend journal "
+					"(err %d)", error);
+				return error;
+			}
+		}
+		ea_inode = ext4_xattr_inode_iget(inode,
+				lea_ino_array->xia_inodes[idx], &error);
+		if (error)
+			continue;
+		ext4_orphan_add(handle, ea_inode);
+		/* the inode's i_count will be released by caller */
+	}
+
+	return 0;
+}
 
 /*
  * ext4_xattr_delete_inode()
  *
- * Free extended attribute resources associated with this inode. This
+ * Free extended attribute resources associated with this inode. Traverse
+ * all entries and unlink any xattr inodes associated with this inode. This
  * is called immediately before an inode is freed. We have exclusive
- * access to the inode.
+ * access to the inode. If an orphan inode is deleted it will also delete any
+ * xattr block and all xattr inodes. They are checked by ext4_xattr_inode_iget()
+ * to ensure they belong to the parent inode and were not deleted already.
  */
-void
-ext4_xattr_delete_inode(handle_t *handle, struct inode *inode)
+int
+ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
+			struct ext4_xattr_ino_array **lea_ino_array)
 {
 	struct buffer_head *bh = NULL;
+	struct ext4_xattr_ibody_header *header;
+	struct ext4_inode *raw_inode;
+	struct ext4_iloc iloc;
+	struct ext4_xattr_entry *entry;
+	int credits = 3, error = 0;
 
-	if (!EXT4_I(inode)->i_file_acl)
+	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
+		goto delete_external_ea;
+
+	error = ext4_get_inode_loc(inode, &iloc);
+	if (error)
+		goto cleanup;
+	raw_inode = ext4_raw_inode(&iloc);
+	header = IHDR(inode, raw_inode);
+	for (entry = IFIRST(header); !IS_LAST_ENTRY(entry);
+	     entry = EXT4_XATTR_NEXT(entry)) {
+		if (!entry->e_value_inum)
+			continue;
+		if (ext4_expand_ino_array(lea_ino_array,
+					  entry->e_value_inum) != 0) {
+			brelse(iloc.bh);
+			goto cleanup;
+		}
+		entry->e_value_inum = 0;
+	}
+	brelse(iloc.bh);
+
+delete_external_ea:
+	if (!EXT4_I(inode)->i_file_acl) {
+		/* add xattr inode to orphan list */
+		ext4_xattr_inode_orphan_add(handle, inode, credits,
+						*lea_ino_array);
 		goto cleanup;
+	}
 	bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
 	if (!bh) {
 		EXT4_ERROR_INODE(inode, "block %llu read error",

@@ -1599,11 +2023,69 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode)
 				 EXT4_I(inode)->i_file_acl);
 		goto cleanup;
 	}
+
+	for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry);
+	     entry = EXT4_XATTR_NEXT(entry)) {
+		if (!entry->e_value_inum)
+			continue;
+		if (ext4_expand_ino_array(lea_ino_array,
+					  entry->e_value_inum) != 0)
+			goto cleanup;
+		entry->e_value_inum = 0;
+	}
+
+	/* add xattr inode to orphan list */
+	error = ext4_xattr_inode_orphan_add(handle, inode, credits,
+					*lea_ino_array);
+	if (error != 0)
+		goto cleanup;
+
+	if (!IS_NOQUOTA(inode))
+		credits += 2 * EXT4_QUOTA_DEL_BLOCKS(inode->i_sb);
+
+	if (!ext4_handle_has_enough_credits(handle, credits)) {
+		error = ext4_journal_extend(handle, credits);
+		if (error > 0)
+			error = ext4_journal_restart(handle, credits);
+		if (error != 0) {
+			ext4_warning(inode->i_sb,
+				"couldn't extend journal (err %d)", error);
+			goto cleanup;
+		}
+	}
+
 	ext4_xattr_release_block(handle, inode, bh);
 	EXT4_I(inode)->i_file_acl = 0;
 
 cleanup:
 	brelse(bh);
+
+	return error;
+}
+
+void
+ext4_xattr_inode_array_free(struct inode *inode,
+			    struct ext4_xattr_ino_array *lea_ino_array)
+{
+	struct inode	*ea_inode = NULL;
+	int		idx = 0;
+	int		err;
+
+	if (lea_ino_array == NULL)
+		return;
+
+	for (; idx < lea_ino_array->xia_count; ++idx) {
+		ea_inode = ext4_xattr_inode_iget(inode,
+				lea_ino_array->xia_inodes[idx], &err);
+		if (err)
+			continue;
+		/* for inode's i_count get from ext4_xattr_delete_inode */
+		if (!list_empty(&EXT4_I(ea_inode)->i_orphan))
+			iput(ea_inode);
+		clear_nlink(ea_inode);
+		iput(ea_inode);
+	}
+	kfree(lea_ino_array);
 }
 
 /*

@@ -1655,10 +2137,9 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
 		    entry1->e_name_index != entry2->e_name_index ||
 		    entry1->e_name_len != entry2->e_name_len ||
 		    entry1->e_value_size != entry2->e_value_size ||
+		    entry1->e_value_inum != entry2->e_value_inum ||
 		    memcmp(entry1->e_name, entry2->e_name, entry1->e_name_len))
 			return 1;
-		if (entry1->e_value_block != 0 || entry2->e_value_block != 0)
-			return -EFSCORRUPTED;
 		if (memcmp((char *)header1 + le16_to_cpu(entry1->e_value_offs),
 			   (char *)header2 + le16_to_cpu(entry2->e_value_offs),
 			   le32_to_cpu(entry1->e_value_size)))

@@ -1730,7 +2211,7 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
 		       *name++;
 	}
 
-	if (entry->e_value_size != 0) {
+	if (!entry->e_value_inum && entry->e_value_size) {
 		__le32 *value = (__le32 *)((char *)header +
 			le16_to_cpu(entry->e_value_offs));
 		for (n = (le32_to_cpu(entry->e_value_size) +

fs/ext4/xattr.h +30 -3

index 099c8b670ef5..6e10ff9393d4 100644
@@ -44,7 +44,7 @@ struct ext4_xattr_entry {
 	__u8	e_name_len;	/* length of name */
 	__u8	e_name_index;	/* attribute name index */
 	__le16	e_value_offs;	/* offset in disk block of value */
-	__le32	e_value_block;	/* disk block attribute is stored on (n/i) */
+	__le32	e_value_inum;	/* inode in which the value is stored */
 	__le32	e_value_size;	/* size of attribute value */
 	__le32	e_hash;		/* hash value of name and value */
 	char	e_name[0];	/* attribute name */

@@ -69,6 +69,26 @@ struct ext4_xattr_entry {
 		EXT4_I(inode)->i_extra_isize))
 #define IFIRST(hdr) ((struct ext4_xattr_entry *)((hdr)+1))
 
+/*
+ * Link EA inode back to parent one using i_mtime field.
+ * Extra integer type conversion added to ignore higher
+ * bits in i_mtime.tv_sec which might be set by ext4_get()
+ */
+#define EXT4_XATTR_INODE_SET_PARENT(inode, inum)      \
+do {                                                  \
+      (inode)->i_mtime.tv_sec = inum;                 \
+} while(0)
+
+#define EXT4_XATTR_INODE_GET_PARENT(inode)            \
+((__u32)(inode)->i_mtime.tv_sec)
+
+/*
+ * The minimum size of EA value when you start storing it in an external inode
+ * size of block - size of header - size of 1 entry - 4 null bytes
+*/
+#define EXT4_XATTR_MIN_LARGE_EA_SIZE(b)					\
+	((b) - EXT4_XATTR_LEN(3) - sizeof(struct ext4_xattr_header) - 4)
+
 #define BHDR(bh) ((struct ext4_xattr_header *)((bh)->b_data))
 #define ENTRY(ptr) ((struct ext4_xattr_entry *)(ptr))
 #define BFIRST(bh) ENTRY(BHDR(bh)+1)

@@ -77,10 +97,11 @@ struct ext4_xattr_entry {
 #define EXT4_ZERO_XATTR_VALUE ((void *)-1)
 
 struct ext4_xattr_info {
-	int name_index;
 	const char *name;
 	const void *value;
 	size_t value_len;
+	int name_index;
+	int in_inode;
 };
 
 struct ext4_xattr_search {

@@ -140,7 +161,13 @@ extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext4_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
 extern int ext4_xattr_set_handle(handle_t *, struct inode *, int, const char *, const void *, size_t, int);
 
-extern void ext4_xattr_delete_inode(handle_t *, struct inode *);
+extern struct inode *ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
+					   int *err);
+extern int ext4_xattr_inode_unlink(struct inode *inode, unsigned long ea_ino);
+extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
+				   struct ext4_xattr_ino_array **array);
+extern void ext4_xattr_inode_array_free(struct inode *inode,
+					struct ext4_xattr_ino_array *array);
 
 extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
 			    struct ext4_inode *raw_inode, handle_t *handle);

Signature · 1 line

2.13.0.219.gdb65acc882-goog

Quoted 1 line

allowing the ext4/e2fsck to verify the correct inode is accessed.

Can we store the checksum of the xattr value somewhere? We already checksum the values if they're stored in the ibody or a single external block, and I'd hate to lose that protection.

We could probably reuse one of the inode fields (i_version?) for this.