Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672238 > unrolled thread

[PATCH 24/32] ext2, ext4: make mb block cache names more explicit

Back to article view | Back to linux.kernel

Contents

  [PATCH 24/32] ext2, ext4: make mb block cache names more explicit Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
    [PATCH 26/32] ext4: cleanup transaction restarts during inode deletion Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
      Re: [PATCH 26/32] ext4: cleanup transaction restarts during inode  deletion Theodore Ts'o <tytso@mit.edu> - 2017-06-22 17:50 +0200
    [PATCH 25/32] ext4: add ext4_is_quota_file() Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
      Re: [PATCH 25/32] ext4: add ext4_is_quota_file() Theodore Ts'o <tytso@mit.edu> - 2017-06-22 17:40 +0200
    [PATCH 32/32] ext4: add nombcache mount option Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
      Re: [PATCH 32/32] ext4: add nombcache mount option Theodore Ts'o <tytso@mit.edu> - 2017-06-22 18:10 +0200
    [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation for removes Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
      Re: [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation  for removes Theodore Ts'o <tytso@mit.edu> - 2017-06-22 18:00 +0200
    [PATCH 31/32] ext4: strong binding of xattr inode references Tahsin Erdogan <tahsin@google.com> - 2017-06-22 04:00 +0200
      Re: [PATCH 31/32] ext4: strong binding of xattr inode references Theodore Ts'o <tytso@mit.edu> - 2017-06-22 18:00 +0200
    Re: [PATCH 24/32] ext2, ext4: make mb block cache names more explicit Theodore Ts'o <tytso@mit.edu> - 2017-06-22 17:40 +0200
    Re: [PATCH 27/32] ext4: xattr inode deduplication Theodore Ts'o <tytso@mit.edu> - 2017-06-22 18:00 +0200

#1672238 — [PATCH 24/32] ext2, ext4: make mb block cache names more explicit

There will be a second mb_cache instance that tracks ea_inodes. Make
existing names more explicit so that it is clear that they refer to
xattr block cache.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v2:
 - renamed s_mb_cache to s_mb_block_cache in both ext2 and ext4
 - renamed local variables named ext[24]_mb_cache to mb_block_cache
 - renamed macro EXT4_GET_MB_CACHE() to MB_BLOCK_CACHE()
 - added MB_BLOCK_CACHE() to ext2 for consistency with ext4

 fs/ext2/ext2.h  |  2 +-
 fs/ext2/super.c | 16 ++++++-------
 fs/ext2/xattr.c | 36 +++++++++++++++--------------
 fs/ext4/ext4.h  |  2 +-
 fs/ext4/super.c | 18 +++++++--------
 fs/ext4/xattr.c | 71 ++++++++++++++++++++++++++++++---------------------------
 6 files changed, 75 insertions(+), 70 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 03f5ce1d3dbe..23ebb92484c6 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -113,7 +113,7 @@ struct ext2_sb_info {
 	 * of the mount options.
 	 */
 	spinlock_t s_lock;
-	struct mb_cache *s_mb_cache;
+	struct mb_cache *s_ea_block_cache;
 };
 
 static inline spinlock_t *
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 9c2028b50e5c..7b1bc9059863 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -147,9 +147,9 @@ static void ext2_put_super (struct super_block * sb)
 
 	ext2_quota_off_umount(sb);
 
-	if (sbi->s_mb_cache) {
-		ext2_xattr_destroy_cache(sbi->s_mb_cache);
-		sbi->s_mb_cache = NULL;
+	if (sbi->s_ea_block_cache) {
+		ext2_xattr_destroy_cache(sbi->s_ea_block_cache);
+		sbi->s_ea_block_cache = NULL;
 	}
 	if (!(sb->s_flags & MS_RDONLY)) {
 		struct ext2_super_block *es = sbi->s_es;
@@ -1131,9 +1131,9 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	}
 
 #ifdef CONFIG_EXT2_FS_XATTR
-	sbi->s_mb_cache = ext2_xattr_create_cache();
-	if (!sbi->s_mb_cache) {
-		ext2_msg(sb, KERN_ERR, "Failed to create an mb_cache");
+	sbi->s_ea_block_cache = ext2_xattr_create_cache();
+	if (!sbi->s_ea_block_cache) {
+		ext2_msg(sb, KERN_ERR, "Failed to create ea_block_cache");
 		goto failed_mount3;
 	}
 #endif
@@ -1182,8 +1182,8 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 			sb->s_id);
 	goto failed_mount;
 failed_mount3:
-	if (sbi->s_mb_cache)
-		ext2_xattr_destroy_cache(sbi->s_mb_cache);
+	if (sbi->s_ea_block_cache)
+		ext2_xattr_destroy_cache(sbi->s_ea_block_cache);
 	percpu_counter_destroy(&sbi->s_freeblocks_counter);
 	percpu_counter_destroy(&sbi->s_freeinodes_counter);
 	percpu_counter_destroy(&sbi->s_dirs_counter);
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 1e5f76070580..1b9b1268d418 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -121,6 +121,8 @@ const struct xattr_handler *ext2_xattr_handlers[] = {
 	NULL
 };
 
+#define EA_BLOCK_CACHE(inode)	(EXT2_SB(inode->i_sb)->s_ea_block_cache)
+
 static inline const struct xattr_handler *
 ext2_xattr_handler(int name_index)
 {
@@ -150,7 +152,7 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
 	size_t name_len, size;
 	char *end;
 	int error;
-	struct mb_cache *ext2_mb_cache = EXT2_SB(inode->i_sb)->s_mb_cache;
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	ea_idebug(inode, "name=%d.%s, buffer=%p, buffer_size=%ld",
 		  name_index, name, buffer, (long)buffer_size);
@@ -195,7 +197,7 @@ bad_block:	ext2_error(inode->i_sb, "ext2_xattr_get",
 			goto found;
 		entry = next;
 	}
-	if (ext2_xattr_cache_insert(ext2_mb_cache, bh))
+	if (ext2_xattr_cache_insert(ea_block_cache, bh))
 		ea_idebug(inode, "cache insert failed");
 	error = -ENODATA;
 	goto cleanup;
@@ -208,7 +210,7 @@ bad_block:	ext2_error(inode->i_sb, "ext2_xattr_get",
 	    le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
 		goto bad_block;
 
-	if (ext2_xattr_cache_insert(ext2_mb_cache, bh))
+	if (ext2_xattr_cache_insert(ea_block_cache, bh))
 		ea_idebug(inode, "cache insert failed");
 	if (buffer) {
 		error = -ERANGE;
@@ -246,7 +248,7 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	char *end;
 	size_t rest = buffer_size;
 	int error;
-	struct mb_cache *ext2_mb_cache = EXT2_SB(inode->i_sb)->s_mb_cache;
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
 		  buffer, (long)buffer_size);
@@ -281,7 +283,7 @@ bad_block:	ext2_error(inode->i_sb, "ext2_xattr_list",
 			goto bad_block;
 		entry = next;
 	}
-	if (ext2_xattr_cache_insert(ext2_mb_cache, bh))
+	if (ext2_xattr_cache_insert(ea_block_cache, bh))
 		ea_idebug(inode, "cache insert failed");
 
 	/* list the attribute names */
@@ -493,7 +495,7 @@ bad_block:		ext2_error(sb, "ext2_xattr_set",
 			 * This must happen under buffer lock for
 			 * ext2_xattr_set2() to reliably detect modified block
 			 */
-			mb_cache_entry_delete(EXT2_SB(sb)->s_mb_cache, hash,
+			mb_cache_entry_delete(EA_BLOCK_CACHE(inode), hash,
 					      bh->b_blocknr);
 
 			/* keep the buffer locked while modifying it. */
@@ -627,7 +629,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 	struct super_block *sb = inode->i_sb;
 	struct buffer_head *new_bh = NULL;
 	int error;
-	struct mb_cache *ext2_mb_cache = EXT2_SB(sb)->s_mb_cache;
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	if (header) {
 		new_bh = ext2_xattr_cache_find(inode, header);
@@ -655,7 +657,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 			   don't need to change the reference count. */
 			new_bh = old_bh;
 			get_bh(new_bh);
-			ext2_xattr_cache_insert(ext2_mb_cache, new_bh);
+			ext2_xattr_cache_insert(ea_block_cache, new_bh);
 		} else {
 			/* We need to allocate a new block */
 			ext2_fsblk_t goal = ext2_group_first_block_no(sb,
@@ -676,7 +678,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 			memcpy(new_bh->b_data, header, new_bh->b_size);
 			set_buffer_uptodate(new_bh);
 			unlock_buffer(new_bh);
-			ext2_xattr_cache_insert(ext2_mb_cache, new_bh);
+			ext2_xattr_cache_insert(ea_block_cache, new_bh);
 			
 			ext2_xattr_update_super_block(sb);
 		}
@@ -721,7 +723,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 			 * This must happen under buffer lock for
 			 * ext2_xattr_set2() to reliably detect freed block
 			 */
-			mb_cache_entry_delete(ext2_mb_cache, hash,
+			mb_cache_entry_delete(ea_block_cache, hash,
 					      old_bh->b_blocknr);
 			/* Free the old block. */
 			ea_bdebug(old_bh, "freeing");
@@ -795,7 +797,7 @@ ext2_xattr_delete_inode(struct inode *inode)
 		 * This must happen under buffer lock for ext2_xattr_set2() to
 		 * reliably detect freed block
 		 */
-		mb_cache_entry_delete(EXT2_SB(inode->i_sb)->s_mb_cache, hash,
+		mb_cache_entry_delete(EA_BLOCK_CACHE(inode), hash,
 				      bh->b_blocknr);
 		ext2_free_blocks(inode, EXT2_I(inode)->i_file_acl, 1);
 		get_bh(bh);
@@ -897,13 +899,13 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
 {
 	__u32 hash = le32_to_cpu(header->h_hash);
 	struct mb_cache_entry *ce;
-	struct mb_cache *ext2_mb_cache = EXT2_SB(inode->i_sb)->s_mb_cache;
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	if (!header->h_hash)
 		return NULL;  /* never share */
 	ea_idebug(inode, "looking for cached blocks [%x]", (int)hash);
 again:
-	ce = mb_cache_entry_find_first(ext2_mb_cache, hash);
+	ce = mb_cache_entry_find_first(ea_block_cache, hash);
 	while (ce) {
 		struct buffer_head *bh;
 
@@ -924,7 +926,7 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
 			 * entry is still hashed is reliable.
 			 */
 			if (hlist_bl_unhashed(&ce->e_hash_list)) {
-				mb_cache_entry_put(ext2_mb_cache, ce);
+				mb_cache_entry_put(ea_block_cache, ce);
 				unlock_buffer(bh);
 				brelse(bh);
 				goto again;
@@ -937,14 +939,14 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
 			} else if (!ext2_xattr_cmp(header, HDR(bh))) {
 				ea_bdebug(bh, "b_count=%d",
 					  atomic_read(&(bh->b_count)));
-				mb_cache_entry_touch(ext2_mb_cache, ce);
-				mb_cache_entry_put(ext2_mb_cache, ce);
+				mb_cache_entry_touch(ea_block_cache, ce);
+				mb_cache_entry_put(ea_block_cache, ce);
 				return bh;
 			}
 			unlock_buffer(bh);
 			brelse(bh);
 		}
-		ce = mb_cache_entry_find_next(ext2_mb_cache, ce);
+		ce = mb_cache_entry_find_next(ea_block_cache, ce);
 	}
 	return NULL;
 }
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 580fdb753f29..e4a75c643731 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1516,7 +1516,7 @@ struct ext4_sb_info {
 	struct list_head s_es_list;	/* List of inodes with reclaimable extents */
 	long s_es_nr_inode;
 	struct ext4_es_stats s_es_stats;
-	struct mb_cache *s_mb_cache;
+	struct mb_cache *s_ea_block_cache;
 	spinlock_t s_es_lock ____cacheline_aligned_in_smp;
 
 	/* Ratelimit ext4 messages. */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b02a23ec92ca..380389740575 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -927,9 +927,9 @@ static void ext4_put_super(struct super_block *sb)
 		invalidate_bdev(sbi->journal_bdev);
 		ext4_blkdev_remove(sbi);
 	}
-	if (sbi->s_mb_cache) {
-		ext4_xattr_destroy_cache(sbi->s_mb_cache);
-		sbi->s_mb_cache = NULL;
+	if (sbi->s_ea_block_cache) {
+		ext4_xattr_destroy_cache(sbi->s_ea_block_cache);
+		sbi->s_ea_block_cache = NULL;
 	}
 	if (sbi->s_mmp_tsk)
 		kthread_stop(sbi->s_mmp_tsk);
@@ -4061,9 +4061,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_journal->j_commit_callback = ext4_journal_commit_callback;
 
 no_journal:
-	sbi->s_mb_cache = ext4_xattr_create_cache();
-	if (!sbi->s_mb_cache) {
-		ext4_msg(sb, KERN_ERR, "Failed to create an mb_cache");
+	sbi->s_ea_block_cache = ext4_xattr_create_cache();
+	if (!sbi->s_ea_block_cache) {
+		ext4_msg(sb, KERN_ERR, "Failed to create ea_block_cache");
 		goto failed_mount_wq;
 	}
 
@@ -4296,9 +4296,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (EXT4_SB(sb)->rsv_conversion_wq)
 		destroy_workqueue(EXT4_SB(sb)->rsv_conversion_wq);
 failed_mount_wq:
-	if (sbi->s_mb_cache) {
-		ext4_xattr_destroy_cache(sbi->s_mb_cache);
-		sbi->s_mb_cache = NULL;
+	if (sbi->s_ea_block_cache) {
+		ext4_xattr_destroy_cache(sbi->s_ea_block_cache);
+		sbi->s_ea_block_cache = NULL;
 	}
 	if (sbi->s_journal) {
 		jbd2_journal_destroy(sbi->s_journal);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index fb437efa8688..fd73dbc92c21 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -72,10 +72,11 @@
 # define ea_bdebug(bh, fmt, ...)	no_printk(fmt, ##__VA_ARGS__)
 #endif
 
-static void ext4_xattr_cache_insert(struct mb_cache *, struct buffer_head *);
-static struct buffer_head *ext4_xattr_cache_find(struct inode *,
-						 struct ext4_xattr_header *,
-						 struct mb_cache_entry **);
+static void ext4_xattr_block_cache_insert(struct mb_cache *,
+					  struct buffer_head *);
+static struct buffer_head *
+ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
+			    struct mb_cache_entry **);
 static void ext4_xattr_rehash(struct ext4_xattr_header *,
 			      struct ext4_xattr_entry *);
 
@@ -104,8 +105,8 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
 	NULL
 };
 
-#define EXT4_GET_MB_CACHE(inode)	(((struct ext4_sb_info *) \
-				inode->i_sb->s_fs_info)->s_mb_cache)
+#define EA_BLOCK_CACHE(inode)	(((struct ext4_sb_info *) \
+				inode->i_sb->s_fs_info)->s_ea_block_cache)
 
 #ifdef CONFIG_LOCKDEP
 void ext4_xattr_inode_set_class(struct inode *ea_inode)
@@ -374,7 +375,7 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 	struct ext4_xattr_entry *entry;
 	size_t size;
 	int error;
-	struct mb_cache *ext4_mb_cache = EXT4_GET_MB_CACHE(inode);
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	ea_idebug(inode, "name=%d.%s, buffer=%p, buffer_size=%ld",
 		  name_index, name, buffer, (long)buffer_size);
@@ -395,7 +396,7 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 		error = -EFSCORRUPTED;
 		goto cleanup;
 	}
-	ext4_xattr_cache_insert(ext4_mb_cache, bh);
+	ext4_xattr_block_cache_insert(ea_block_cache, bh);
 	entry = BFIRST(bh);
 	error = ext4_xattr_find_entry(&entry, name_index, name, 1);
 	if (error)
@@ -541,7 +542,6 @@ ext4_xattr_block_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 	struct inode *inode = d_inode(dentry);
 	struct buffer_head *bh = NULL;
 	int error;
-	struct mb_cache *ext4_mb_cache = EXT4_GET_MB_CACHE(inode);
 
 	ea_idebug(inode, "buffer=%p, buffer_size=%ld",
 		  buffer, (long)buffer_size);
@@ -563,7 +563,7 @@ ext4_xattr_block_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 		error = -EFSCORRUPTED;
 		goto cleanup;
 	}
-	ext4_xattr_cache_insert(ext4_mb_cache, bh);
+	ext4_xattr_block_cache_insert(EA_BLOCK_CACHE(inode), bh);
 	error = ext4_xattr_list_entries(dentry, BFIRST(bh), buffer, buffer_size);
 
 cleanup:
@@ -660,7 +660,7 @@ static void
 ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 			 struct buffer_head *bh)
 {
-	struct mb_cache *ext4_mb_cache = EXT4_GET_MB_CACHE(inode);
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 	u32 hash, ref;
 	int error = 0;
 
@@ -678,7 +678,7 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 		 * This must happen under buffer lock for
 		 * ext4_xattr_block_set() to reliably detect freed block
 		 */
-		mb_cache_entry_delete(ext4_mb_cache, hash, bh->b_blocknr);
+		mb_cache_entry_delete(ea_block_cache, hash, bh->b_blocknr);
 		get_bh(bh);
 		unlock_buffer(bh);
 		ext4_free_blocks(handle, inode, bh, 0, 1,
@@ -690,11 +690,11 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 		if (ref == EXT4_XATTR_REFCOUNT_MAX - 1) {
 			struct mb_cache_entry *ce;
 
-			ce = mb_cache_entry_get(ext4_mb_cache, hash,
+			ce = mb_cache_entry_get(ea_block_cache, hash,
 						bh->b_blocknr);
 			if (ce) {
 				ce->e_reusable = 1;
-				mb_cache_entry_put(ext4_mb_cache, ce);
+				mb_cache_entry_put(ea_block_cache, ce);
 			}
 		}
 
@@ -1096,7 +1096,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 	struct ext4_xattr_search *s = &s_copy;
 	struct mb_cache_entry *ce = NULL;
 	int error = 0;
-	struct mb_cache *ext4_mb_cache = EXT4_GET_MB_CACHE(inode);
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 #define header(x) ((struct ext4_xattr_header *)(x))
 
@@ -1115,7 +1115,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			 * ext4_xattr_block_set() to reliably detect modified
 			 * block
 			 */
-			mb_cache_entry_delete(ext4_mb_cache, hash,
+			mb_cache_entry_delete(ea_block_cache, hash,
 					      bs->bh->b_blocknr);
 			ea_bdebug(bs->bh, "modifying in-place");
 			error = ext4_xattr_set_entry(i, s, handle, inode);
@@ -1123,8 +1123,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 				if (!IS_LAST_ENTRY(s->first))
 					ext4_xattr_rehash(header(s->base),
 							  s->here);
-				ext4_xattr_cache_insert(ext4_mb_cache,
-					bs->bh);
+				ext4_xattr_block_cache_insert(ea_block_cache,
+							      bs->bh);
 			}
 			ext4_xattr_block_csum_set(inode, bs->bh);
 			unlock_buffer(bs->bh);
@@ -1177,7 +1177,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 
 inserted:
 	if (!IS_LAST_ENTRY(s->first)) {
-		new_bh = ext4_xattr_cache_find(inode, header(s->base), &ce);
+		new_bh = ext4_xattr_block_cache_find(inode, header(s->base),
+						     &ce);
 		if (new_bh) {
 			/* We found an identical block in the cache. */
 			if (new_bh == bs->bh)
@@ -1222,7 +1223,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 							 EXT4_C2B(EXT4_SB(sb),
 								  1));
 					brelse(new_bh);
-					mb_cache_entry_put(ext4_mb_cache, ce);
+					mb_cache_entry_put(ea_block_cache, ce);
 					ce = NULL;
 					new_bh = NULL;
 					goto inserted;
@@ -1241,8 +1242,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 				if (error)
 					goto cleanup_dquot;
 			}
-			mb_cache_entry_touch(ext4_mb_cache, ce);
-			mb_cache_entry_put(ext4_mb_cache, ce);
+			mb_cache_entry_touch(ea_block_cache, ce);
+			mb_cache_entry_put(ea_block_cache, ce);
 			ce = NULL;
 		} else if (bs->bh && s->base == bs->bh->b_data) {
 			/* We were modifying this block in-place. */
@@ -1292,7 +1293,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			ext4_xattr_block_csum_set(inode, new_bh);
 			set_buffer_uptodate(new_bh);
 			unlock_buffer(new_bh);
-			ext4_xattr_cache_insert(ext4_mb_cache, new_bh);
+			ext4_xattr_block_cache_insert(ea_block_cache, new_bh);
 			error = ext4_handle_dirty_metadata(handle, inode,
 							   new_bh);
 			if (error)
@@ -1310,7 +1311,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 
 cleanup:
 	if (ce)
-		mb_cache_entry_put(ext4_mb_cache, ce);
+		mb_cache_entry_put(ea_block_cache, ce);
 	brelse(new_bh);
 	if (!(bs->bh && s->base == bs->bh->b_data))
 		kfree(s->base);
@@ -2150,15 +2151,16 @@ void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *ea_inode_array)
 }
 
 /*
- * ext4_xattr_cache_insert()
+ * ext4_xattr_block_cache_insert()
  *
- * Create a new entry in the extended attribute cache, and insert
+ * Create a new entry in the extended attribute block cache, and insert
  * it unless such an entry is already in the cache.
  *
  * Returns 0, or a negative error number on failure.
  */
 static void
-ext4_xattr_cache_insert(struct mb_cache *ext4_mb_cache, struct buffer_head *bh)
+ext4_xattr_block_cache_insert(struct mb_cache *ea_block_cache,
+			      struct buffer_head *bh)
 {
 	struct ext4_xattr_header *header = BHDR(bh);
 	__u32 hash = le32_to_cpu(header->h_hash);
@@ -2166,7 +2168,7 @@ ext4_xattr_cache_insert(struct mb_cache *ext4_mb_cache, struct buffer_head *bh)
 		       EXT4_XATTR_REFCOUNT_MAX;
 	int error;
 
-	error = mb_cache_entry_create(ext4_mb_cache, GFP_NOFS, hash,
+	error = mb_cache_entry_create(ea_block_cache, GFP_NOFS, hash,
 				      bh->b_blocknr, reusable);
 	if (error) {
 		if (error == -EBUSY)
@@ -2216,7 +2218,7 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
 }
 
 /*
- * ext4_xattr_cache_find()
+ * ext4_xattr_block_cache_find()
  *
  * Find an identical extended attribute block.
  *
@@ -2224,17 +2226,18 @@ ext4_xattr_cmp(struct ext4_xattr_header *header1,
  * not found or an error occurred.
  */
 static struct buffer_head *
-ext4_xattr_cache_find(struct inode *inode, struct ext4_xattr_header *header,
-		      struct mb_cache_entry **pce)
+ext4_xattr_block_cache_find(struct inode *inode,
+			    struct ext4_xattr_header *header,
+			    struct mb_cache_entry **pce)
 {
 	__u32 hash = le32_to_cpu(header->h_hash);
 	struct mb_cache_entry *ce;
-	struct mb_cache *ext4_mb_cache = EXT4_GET_MB_CACHE(inode);
+	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
 	if (!header->h_hash)
 		return NULL;  /* never share */
 	ea_idebug(inode, "looking for cached blocks [%x]", (int)hash);
-	ce = mb_cache_entry_find_first(ext4_mb_cache, hash);
+	ce = mb_cache_entry_find_first(ea_block_cache, hash);
 	while (ce) {
 		struct buffer_head *bh;
 
@@ -2247,7 +2250,7 @@ ext4_xattr_cache_find(struct inode *inode, struct ext4_xattr_header *header,
 			return bh;
 		}
 		brelse(bh);
-		ce = mb_cache_entry_find_next(ext4_mb_cache, ce);
+		ce = mb_cache_entry_find_next(ea_block_cache, ce);
 	}
 	return NULL;
 }
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [next] | [standalone]

#1672239 — [PATCH 26/32] ext4: cleanup transaction restarts during inode deletion

During inode deletion, journal credits that will be needed are hard to
determine, that is why we have journal extend/restart calls in several
places. Whenever a transaction is restarted, filesystem must be in a
consistent state because there is no atomicity guarantee beyond a
restart call.

Add ext4_xattr_ensure_credits() helper function which takes care of
journal extend/restart logic. It also handles getting jbd2 write access
and dirty metadata calls. This function is called at every iteration of
handling an ea_inode reference.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v3: fixed checkpatch.pl warnings about long lines and indented label 

v2: made ext4_xattr_ensure_credits() static

 fs/ext4/inode.c |  66 ++++-----------
 fs/ext4/xattr.c | 258 ++++++++++++++++++++++++++++++++++++--------------------
 fs/ext4/xattr.h |   3 +-
 3 files changed, 184 insertions(+), 143 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index cf91532765a4..cd007f9757d1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -239,7 +239,11 @@ void ext4_evict_inode(struct inode *inode)
 	 */
 	sb_start_intwrite(inode->i_sb);
 
-	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, extra_credits);
+	if (!IS_NOQUOTA(inode))
+		extra_credits += EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb);
+
+	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE,
+				 ext4_blocks_for_truncate(inode)+extra_credits);
 	if (IS_ERR(handle)) {
 		ext4_std_error(inode->i_sb, PTR_ERR(handle));
 		/*
@@ -251,36 +255,9 @@ 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, &ea_inode_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) {
@@ -298,25 +275,17 @@ void ext4_evict_inode(struct inode *inode)
 		}
 	}
 
-	/*
-	 * ext4_ext_truncate() doesn't reserve any slop when it
-	 * restarts journal transactions; therefore there may not be
-	 * 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, extra_credits)) {
-		err = ext4_journal_extend(handle, extra_credits);
-		if (err > 0)
-			err = ext4_journal_restart(handle, extra_credits);
-		if (err != 0) {
-			ext4_warning(inode->i_sb,
-				     "couldn't extend journal (err %d)", err);
-		stop_handle:
-			ext4_journal_stop(handle);
-			ext4_orphan_del(NULL, inode);
-			sb_end_intwrite(inode->i_sb);
-			goto no_delete;
-		}
+	/* Remove xattr references. */
+	err = ext4_xattr_delete_inode(handle, inode, &ea_inode_array,
+				      extra_credits);
+	if (err) {
+		ext4_warning(inode->i_sb, "xattr delete (err %d)", err);
+stop_handle:
+		ext4_journal_stop(handle);
+		ext4_orphan_del(NULL, inode);
+		sb_end_intwrite(inode->i_sb);
+		ext4_xattr_inode_array_free(ea_inode_array);
+		goto no_delete;
 	}
 
 	/*
@@ -342,7 +311,6 @@ 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);
 	ext4_xattr_inode_array_free(ea_inode_array);
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index fd73dbc92c21..94f04b9fb421 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -108,6 +108,10 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
 #define EA_BLOCK_CACHE(inode)	(((struct ext4_sb_info *) \
 				inode->i_sb->s_fs_info)->s_ea_block_cache)
 
+static int
+ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array,
+			struct inode *inode);
+
 #ifdef CONFIG_LOCKDEP
 void ext4_xattr_inode_set_class(struct inode *ea_inode)
 {
@@ -652,6 +656,128 @@ static void ext4_xattr_update_super_block(handle_t *handle,
 	}
 }
 
+static int ext4_xattr_ensure_credits(handle_t *handle, struct inode *inode,
+				     int credits, struct buffer_head *bh,
+				     bool dirty, bool block_csum)
+{
+	int error;
+
+	if (!ext4_handle_valid(handle))
+		return 0;
+
+	if (handle->h_buffer_credits >= credits)
+		return 0;
+
+	error = ext4_journal_extend(handle, credits - handle->h_buffer_credits);
+	if (!error)
+		return 0;
+	if (error < 0) {
+		ext4_warning(inode->i_sb, "Extend journal (error %d)", error);
+		return error;
+	}
+
+	if (bh && dirty) {
+		if (block_csum)
+			ext4_xattr_block_csum_set(inode, bh);
+		error = ext4_handle_dirty_metadata(handle, NULL, bh);
+		if (error) {
+			ext4_warning(inode->i_sb, "Handle metadata (error %d)",
+				     error);
+			return error;
+		}
+	}
+
+	error = ext4_journal_restart(handle, credits);
+	if (error) {
+		ext4_warning(inode->i_sb, "Restart journal (error %d)", error);
+		return error;
+	}
+
+	if (bh) {
+		error = ext4_journal_get_write_access(handle, bh);
+		if (error) {
+			ext4_warning(inode->i_sb,
+				     "Get write access failed (error %d)",
+				     error);
+			return error;
+		}
+	}
+	return 0;
+}
+
+static void
+ext4_xattr_inode_remove_all(handle_t *handle, struct inode *parent,
+			    struct buffer_head *bh,
+			    struct ext4_xattr_entry *first, bool block_csum,
+			    struct ext4_xattr_inode_array **ea_inode_array,
+			    int extra_credits)
+{
+	struct inode *ea_inode;
+	struct ext4_xattr_entry *entry;
+	bool dirty = false;
+	unsigned int ea_ino;
+	int err;
+	int credits;
+
+	/* One credit for dec ref on ea_inode, one for orphan list addition, */
+	credits = 2 + extra_credits;
+
+	for (entry = first; !IS_LAST_ENTRY(entry);
+	     entry = EXT4_XATTR_NEXT(entry)) {
+		if (!entry->e_value_inum)
+			continue;
+		ea_ino = le32_to_cpu(entry->e_value_inum);
+		err = ext4_xattr_inode_iget(parent, ea_ino, &ea_inode);
+		if (err)
+			continue;
+
+		err = ext4_expand_inode_array(ea_inode_array, ea_inode);
+		if (err) {
+			ext4_warning_inode(ea_inode,
+					   "Expand inode array err=%d", err);
+			iput(ea_inode);
+			continue;
+		}
+
+		err = ext4_xattr_ensure_credits(handle, parent, credits, bh,
+						dirty, block_csum);
+		if (err) {
+			ext4_warning_inode(ea_inode, "Ensure credits err=%d",
+					   err);
+			continue;
+		}
+
+		inode_lock(ea_inode);
+		clear_nlink(ea_inode);
+		ext4_orphan_add(handle, ea_inode);
+		inode_unlock(ea_inode);
+
+		/*
+		 * Forget about ea_inode within the same transaction that
+		 * decrements the ref count. This avoids duplicate decrements in
+		 * case the rest of the work spills over to subsequent
+		 * transactions.
+		 */
+		entry->e_value_inum = 0;
+		entry->e_value_size = 0;
+
+		dirty = true;
+	}
+
+	if (dirty) {
+		/*
+		 * Note that we are deliberately skipping csum calculation for
+		 * the final update because we do not expect any journal
+		 * restarts until xattr block is freed.
+		 */
+
+		err = ext4_handle_dirty_metadata(handle, NULL, bh);
+		if (err)
+			ext4_warning_inode(parent,
+					   "handle dirty metadata err=%d", err);
+	}
+}
+
 /*
  * Release the xattr block BH: If the reference count is > 1, decrement it;
  * otherwise free the block.
@@ -1984,42 +2110,6 @@ ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array,
 	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_inode_array *ea_inode_array)
-{
-	int idx = 0, error = 0;
-	struct inode *ea_inode;
-
-	if (ea_inode_array == NULL)
-		return 0;
-
-	for (; idx < ea_inode_array->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 = ea_inode_array->inodes[idx];
-		inode_lock(ea_inode);
-		ext4_orphan_add(handle, ea_inode);
-		inode_unlock(ea_inode);
-		/* the inode's i_count will be released by caller */
-	}
-
-	return 0;
-}
-
 /*
  * ext4_xattr_delete_inode()
  *
@@ -2032,16 +2122,23 @@ ext4_xattr_inode_orphan_add(handle_t *handle, struct inode *inode, int credits,
  */
 int
 ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
-			struct ext4_xattr_inode_array **ea_inode_array)
+			struct ext4_xattr_inode_array **ea_inode_array,
+			int extra_credits)
 {
 	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;
-	struct inode *ea_inode;
-	unsigned int ea_ino;
-	int credits = 3, error = 0;
+	struct ext4_iloc iloc = { .bh = NULL };
+	int error;
+
+	error = ext4_xattr_ensure_credits(handle, inode, extra_credits,
+					  NULL /* bh */,
+					  false /* dirty */,
+					  false /* block_csum */);
+	if (error) {
+		EXT4_ERROR_INODE(inode, "ensure credits (error %d)", error);
+		goto cleanup;
+	}
 
 	if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR))
 		goto delete_external_ea;
@@ -2049,31 +2146,20 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
 	error = ext4_get_inode_loc(inode, &iloc);
 	if (error)
 		goto cleanup;
+
+	error = ext4_journal_get_write_access(handle, iloc.bh);
+	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;
-		ea_ino = le32_to_cpu(entry->e_value_inum);
-		error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
-		if (error)
-			continue;
-		error = ext4_expand_inode_array(ea_inode_array, ea_inode);
-		if (error) {
-			iput(ea_inode);
-			brelse(iloc.bh);
-			goto cleanup;
-		}
-		entry->e_value_inum = 0;
-	}
-	brelse(iloc.bh);
+	ext4_xattr_inode_remove_all(handle, inode, iloc.bh, IFIRST(header),
+				    false /* block_csum */, ea_inode_array,
+				    extra_credits);
 
 delete_external_ea:
 	if (!EXT4_I(inode)->i_file_acl) {
-		/* add xattr inode to orphan list */
-		error = ext4_xattr_inode_orphan_add(handle, inode, credits,
-						    *ea_inode_array);
+		error = 0;
 		goto cleanup;
 	}
 	bh = sb_bread(inode->i_sb, EXT4_I(inode)->i_file_acl);
@@ -2091,46 +2177,32 @@ ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
 		goto cleanup;
 	}
 
-	for (entry = BFIRST(bh); !IS_LAST_ENTRY(entry);
-	     entry = EXT4_XATTR_NEXT(entry)) {
-		if (!entry->e_value_inum)
-			continue;
-		ea_ino = le32_to_cpu(entry->e_value_inum);
-		error = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
-		if (error)
-			continue;
-		error = ext4_expand_inode_array(ea_inode_array, ea_inode);
-		if (error)
-			goto cleanup;
-		entry->e_value_inum = 0;
-	}
-
-	/* add xattr inode to orphan list */
-	error = ext4_xattr_inode_orphan_add(handle, inode, credits,
-					*ea_inode_array);
-	if (error)
-		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 (ext4_has_feature_ea_inode(inode->i_sb)) {
+		error = ext4_journal_get_write_access(handle, bh);
 		if (error) {
-			ext4_warning(inode->i_sb,
-				"couldn't extend journal (err %d)", error);
+			EXT4_ERROR_INODE(inode, "write access %llu",
+					 EXT4_I(inode)->i_file_acl);
 			goto cleanup;
 		}
+		ext4_xattr_inode_remove_all(handle, inode, bh,
+					    BFIRST(bh),
+					    true /* block_csum */,
+					    ea_inode_array,
+					    extra_credits);
 	}
 
 	ext4_xattr_release_block(handle, inode, bh);
+	/* Update i_file_acl within the same transaction that releases block. */
 	EXT4_I(inode)->i_file_acl = 0;
-
+	error = ext4_mark_inode_dirty(handle, inode);
+	if (error) {
+		EXT4_ERROR_INODE(inode, "mark inode dirty (error %d)",
+				 error);
+		goto cleanup;
+	}
 cleanup:
+	brelse(iloc.bh);
 	brelse(bh);
-
 	return error;
 }
 
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index adf761518a73..b2005a2716d9 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -169,7 +169,8 @@ extern int ext4_xattr_set_credits(struct inode *inode, size_t value_len);
 
 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_inode_array **array);
+				   struct ext4_xattr_inode_array **array,
+				   int extra_credits);
 extern void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *array);
 
 extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [prev] | [next] | [standalone]

#1672763 — Re: [PATCH 26/32] ext4: cleanup transaction restarts during inode deletion

On Wed, Jun 21, 2017 at 06:49:33PM -0700, Tahsin Erdogan wrote:
> During inode deletion, journal credits that will be needed are hard to
> determine, that is why we have journal extend/restart calls in several
> places. Whenever a transaction is restarted, filesystem must be in a
> consistent state because there is no atomicity guarantee beyond a
> restart call.
> 
> Add ext4_xattr_ensure_credits() helper function which takes care of
> journal extend/restart logic. It also handles getting jbd2 write access
> and dirty metadata calls. This function is called at every iteration of
> handling an ea_inode reference.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Applied, thanks.

					- Ted

[toc] | [prev] | [next] | [standalone]

#1672240 — [PATCH 25/32] ext4: add ext4_is_quota_file()

IS_NOQUOTA() indicates whether quota is disabled for an inode. Ext4
also uses it to check whether an inode is for a quota file. The
distinction currently doesn't matter because quota is disabled only
for the quota files. When we start disabling quota for other inodes
in the future, we will want to make the distinction clear.

Replace IS_NOQUOTA() call with ext4_is_quota_file() at places where
we are checking for quota files.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 fs/ext4/ext4.h        | 2 ++
 fs/ext4/inode.c       | 2 +-
 fs/ext4/ioctl.c       | 4 ++--
 fs/ext4/mballoc.c     | 2 +-
 fs/ext4/move_extent.c | 2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e4a75c643731..3b02bd897b61 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2099,6 +2099,8 @@ static inline struct ext4_inode *ext4_raw_inode(struct ext4_iloc *iloc)
 	return (struct ext4_inode *) (iloc->bh->b_data + iloc->offset);
 }
 
+#define ext4_is_quota_file(inode) IS_NOQUOTA(inode)
+
 /*
  * This structure is stuffed into the struct file's private_data field
  * for directories.  It is where we put information so that we can do
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8ee20b586567..cf91532765a4 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -739,7 +739,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 		if (map->m_flags & EXT4_MAP_NEW &&
 		    !(map->m_flags & EXT4_MAP_UNWRITTEN) &&
 		    !(flags & EXT4_GET_BLOCKS_ZERO) &&
-		    !IS_NOQUOTA(inode) &&
+		    !ext4_is_quota_file(inode) &&
 		    ext4_should_order_data(inode)) {
 			if (flags & EXT4_GET_BLOCKS_IO_SUBMIT)
 				ret = ext4_jbd2_inode_add_wait(handle, inode);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 0c21e22acd74..dde8deb11e59 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -218,7 +218,7 @@ static int ext4_ioctl_setflags(struct inode *inode,
 	unsigned int jflag;
 
 	/* Is it quota file? Do not allow user to mess with it */
-	if (IS_NOQUOTA(inode))
+	if (ext4_is_quota_file(inode))
 		goto flags_out;
 
 	oldflags = ei->i_flags;
@@ -342,7 +342,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
 	err = -EPERM;
 	inode_lock(inode);
 	/* Is it quota file? Do not allow user to mess with it */
-	if (IS_NOQUOTA(inode))
+	if (ext4_is_quota_file(inode))
 		goto out_unlock;
 
 	err = ext4_get_inode_loc(inode, &iloc);
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b7928cddd539..d109a2a2fea0 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4464,7 +4464,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
 	trace_ext4_request_blocks(ar);
 
 	/* Allow to use superuser reservation for quota file */
-	if (IS_NOQUOTA(ar->inode))
+	if (ext4_is_quota_file(ar->inode))
 		ar->flags |= EXT4_MB_USE_ROOT_BLOCKS;
 
 	if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0) {
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index c992ef2c2f94..9bb36909ec92 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -484,7 +484,7 @@ mext_check_arguments(struct inode *orig_inode,
 		return -EBUSY;
 	}
 
-	if (IS_NOQUOTA(orig_inode) || IS_NOQUOTA(donor_inode)) {
+	if (ext4_is_quota_file(orig_inode) && ext4_is_quota_file(donor_inode)) {
 		ext4_debug("ext4 move extent: The argument files should "
 			"not be quota files [ino:orig %lu, donor %lu]\n",
 			orig_inode->i_ino, donor_inode->i_ino);
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [prev] | [next] | [standalone]

#1672758 — Re: [PATCH 25/32] ext4: add ext4_is_quota_file()

On Wed, Jun 21, 2017 at 06:49:32PM -0700, Tahsin Erdogan wrote:
> IS_NOQUOTA() indicates whether quota is disabled for an inode. Ext4
> also uses it to check whether an inode is for a quota file. The
> distinction currently doesn't matter because quota is disabled only
> for the quota files. When we start disabling quota for other inodes
> in the future, we will want to make the distinction clear.
> 
> Replace IS_NOQUOTA() call with ext4_is_quota_file() at places where
> we are checking for quota files.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Added to the ext4 patch queue, thanks.

				- Ted

[toc] | [prev] | [next] | [standalone]

#1672242 — [PATCH 32/32] ext4: add nombcache mount option

The main purpose of mb cache is to achieve deduplication in
extended attributes. In use cases where opportunity for deduplication
is unlikely, it only adds overhead.

Add a mount option to explicitly turn off mb cache.

Suggested-by: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v2:
 - updated definition of EXT4_MOUNT_NO_MBCACHE to be 0x00001

 fs/ext4/ext4.h  |  1 +
 fs/ext4/super.c | 34 +++++++++++++++++++++++-----------
 fs/ext4/xattr.c | 52 +++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index dc06287ddec8..7d66e1dade45 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1114,6 +1114,7 @@ struct ext4_inode_info {
 /*
  * Mount flags set via mount options or defaults
  */
+#define EXT4_MOUNT_NO_MBCACHE		0x00001 /* Do not use mbcache */
 #define EXT4_MOUNT_GRPID		0x00004	/* Create files with directory's group */
 #define EXT4_MOUNT_DEBUG		0x00008	/* Some debugging messages */
 #define EXT4_MOUNT_ERRORS_CONT		0x00010	/* Continue on errors */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 5ac76e8d4013..1fec35bd4084 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1336,7 +1336,7 @@ enum {
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
 	Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable,
-	Opt_max_dir_size_kb, Opt_nojournal_checksum,
+	Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache,
 };
 
 static const match_table_t tokens = {
@@ -1419,6 +1419,8 @@ static const match_table_t tokens = {
 	{Opt_noinit_itable, "noinit_itable"},
 	{Opt_max_dir_size_kb, "max_dir_size_kb=%u"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption"},
+	{Opt_nombcache, "nombcache"},
+	{Opt_nombcache, "no_mbcache"},	/* for backward compatibility */
 	{Opt_removed, "check=none"},	/* mount option from ext2/3 */
 	{Opt_removed, "nocheck"},	/* mount option from ext2/3 */
 	{Opt_removed, "reservation"},	/* mount option from ext2/3 */
@@ -1626,6 +1628,7 @@ static const struct mount_opts {
 	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
 	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
 	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
+	{Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
 	{Opt_err, 0, 0}
 };
 
@@ -4080,19 +4083,22 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	sbi->s_journal->j_commit_callback = ext4_journal_commit_callback;
 
 no_journal:
-	sbi->s_ea_block_cache = ext4_xattr_create_cache();
-	if (!sbi->s_ea_block_cache) {
-		ext4_msg(sb, KERN_ERR, "Failed to create ea_block_cache");
-		goto failed_mount_wq;
-	}
-
-	if (ext4_has_feature_ea_inode(sb)) {
-		sbi->s_ea_inode_cache = ext4_xattr_create_cache();
-		if (!sbi->s_ea_inode_cache) {
+	if (!test_opt(sb, NO_MBCACHE)) {
+		sbi->s_ea_block_cache = ext4_xattr_create_cache();
+		if (!sbi->s_ea_block_cache) {
 			ext4_msg(sb, KERN_ERR,
-				 "Failed to create ea_inode_cache");
+				 "Failed to create ea_block_cache");
 			goto failed_mount_wq;
 		}
+
+		if (ext4_has_feature_ea_inode(sb)) {
+			sbi->s_ea_inode_cache = ext4_xattr_create_cache();
+			if (!sbi->s_ea_inode_cache) {
+				ext4_msg(sb, KERN_ERR,
+					 "Failed to create ea_inode_cache");
+				goto failed_mount_wq;
+			}
+		}
 	}
 
 	if ((DUMMY_ENCRYPTION_ENABLED(sbi) || ext4_has_feature_encrypt(sb)) &&
@@ -4989,6 +4995,12 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		}
 	}
 
+	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_NO_MBCACHE) {
+		ext4_msg(sb, KERN_ERR, "can't enable nombcache during remount");
+		err = -EINVAL;
+		goto restore_opts;
+	}
+
 	if ((sbi->s_mount_opt ^ old_opts.s_mount_opt) & EXT4_MOUNT_DAX) {
 		ext4_msg(sb, KERN_WARNING, "warning: refusing change of "
 			"dax flag with busy inodes while remounting");
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 3f16dc979012..ce12c3fb7e59 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -976,10 +976,13 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 			set_nlink(ea_inode, 1);
 			ext4_orphan_del(handle, ea_inode);
 
-			hash = ext4_xattr_inode_get_hash(ea_inode);
-			mb_cache_entry_create(ea_inode_cache, GFP_NOFS, hash,
-					      ea_inode->i_ino,
-					      true /* reusable */);
+			if (ea_inode_cache) {
+				hash = ext4_xattr_inode_get_hash(ea_inode);
+				mb_cache_entry_create(ea_inode_cache,
+						      GFP_NOFS, hash,
+						      ea_inode->i_ino,
+						      true /* reusable */);
+			}
 		}
 	} else {
 		WARN_ONCE(ref_count < 0, "EA inode %lu ref_count=%lld",
@@ -993,9 +996,11 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 			clear_nlink(ea_inode);
 			ext4_orphan_add(handle, ea_inode);
 
-			hash = ext4_xattr_inode_get_hash(ea_inode);
-			mb_cache_entry_delete(ea_inode_cache, hash,
-					      ea_inode->i_ino);
+			if (ea_inode_cache) {
+				hash = ext4_xattr_inode_get_hash(ea_inode);
+				mb_cache_entry_delete(ea_inode_cache, hash,
+						      ea_inode->i_ino);
+			}
 		}
 	}
 
@@ -1179,7 +1184,9 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 		 * This must happen under buffer lock for
 		 * ext4_xattr_block_set() to reliably detect freed block
 		 */
-		mb_cache_entry_delete(ea_block_cache, hash, bh->b_blocknr);
+		if (ea_block_cache)
+			mb_cache_entry_delete(ea_block_cache, hash,
+					      bh->b_blocknr);
 		get_bh(bh);
 		unlock_buffer(bh);
 
@@ -1199,11 +1206,13 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode,
 		if (ref == EXT4_XATTR_REFCOUNT_MAX - 1) {
 			struct mb_cache_entry *ce;
 
-			ce = mb_cache_entry_get(ea_block_cache, hash,
-						bh->b_blocknr);
-			if (ce) {
-				ce->e_reusable = 1;
-				mb_cache_entry_put(ea_block_cache, ce);
+			if (ea_block_cache) {
+				ce = mb_cache_entry_get(ea_block_cache, hash,
+							bh->b_blocknr);
+				if (ce) {
+					ce->e_reusable = 1;
+					mb_cache_entry_put(ea_block_cache, ce);
+				}
 			}
 		}
 
@@ -1382,6 +1391,9 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 	struct mb_cache *ea_inode_cache = EA_INODE_CACHE(inode);
 	void *ea_data;
 
+	if (!ea_inode_cache)
+		return NULL;
+
 	ce = mb_cache_entry_find_first(ea_inode_cache, hash);
 	if (!ce)
 		return NULL;
@@ -1452,8 +1464,9 @@ static int ext4_xattr_inode_lookup_create(handle_t *handle, struct inode *inode,
 		return err;
 	}
 
-	mb_cache_entry_create(EA_INODE_CACHE(inode), GFP_NOFS, hash,
-			      ea_inode->i_ino, true /* reusable */);
+	if (EA_INODE_CACHE(inode))
+		mb_cache_entry_create(EA_INODE_CACHE(inode), GFP_NOFS, hash,
+				      ea_inode->i_ino, true /* reusable */);
 
 	*ret_inode = ea_inode;
 	return 0;
@@ -1780,8 +1793,9 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			 * ext4_xattr_block_set() to reliably detect modified
 			 * block
 			 */
-			mb_cache_entry_delete(ea_block_cache, hash,
-					      bs->bh->b_blocknr);
+			if (ea_block_cache)
+				mb_cache_entry_delete(ea_block_cache, hash,
+						      bs->bh->b_blocknr);
 			ea_bdebug(bs->bh, "modifying in-place");
 			error = ext4_xattr_set_entry(i, s, handle, inode,
 						     true /* is_block */);
@@ -2870,6 +2884,8 @@ ext4_xattr_block_cache_insert(struct mb_cache *ea_block_cache,
 		       EXT4_XATTR_REFCOUNT_MAX;
 	int error;
 
+	if (!ea_block_cache)
+		return;
 	error = mb_cache_entry_create(ea_block_cache, GFP_NOFS, hash,
 				      bh->b_blocknr, reusable);
 	if (error) {
@@ -2936,6 +2952,8 @@ ext4_xattr_block_cache_find(struct inode *inode,
 	struct mb_cache_entry *ce;
 	struct mb_cache *ea_block_cache = EA_BLOCK_CACHE(inode);
 
+	if (!ea_block_cache)
+		return NULL;
 	if (!header->h_hash)
 		return NULL;  /* never share */
 	ea_idebug(inode, "looking for cached blocks [%x]", (int)hash);
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [prev] | [next] | [standalone]

#1672779 — Re: [PATCH 32/32] ext4: add nombcache mount option

On Wed, Jun 21, 2017 at 06:49:39PM -0700, Tahsin Erdogan wrote:
> The main purpose of mb cache is to achieve deduplication in
> extended attributes. In use cases where opportunity for deduplication
> is unlikely, it only adds overhead.
> 
> Add a mount option to explicitly turn off mb cache.
> 
> Suggested-by: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Applied, thanks.

						- Ted

[toc] | [prev] | [next] | [standalone]

#1672243 — [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation for removes

When an extended attribute block is modified, ext4_xattr_hash_entry()
recalculates e_hash for the entry that is pointed by s->here. This is
unnecessary if the modification is to remove an entry.

Currently, if the removed entry is the last one and there are other
entries remaining, hash calculation targets the just erased entry which
has been filled with zeroes and effectively does nothing. If the removed
entry is not the last one and there are more entries, this time it will
recalculate hash on the next entry which is totally unnecessary.

Fix these by moving the decision on when to recalculate hash to
ext4_xattr_set_entry().

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
 fs/ext4/xattr.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 835b3dca5b65..9cb15f2069bd 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -77,8 +77,9 @@ static void ext4_xattr_block_cache_insert(struct mb_cache *,
 static struct buffer_head *
 ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
 			    struct mb_cache_entry **);
-static void ext4_xattr_rehash(struct ext4_xattr_header *,
-			      struct ext4_xattr_entry *);
+static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
+				  void *value_base);
+static void ext4_xattr_rehash(struct ext4_xattr_header *);
 
 static const struct xattr_handler * const ext4_xattr_handler_map[] = {
 	[EXT4_XATTR_INDEX_USER]		     = &ext4_xattr_user_handler,
@@ -1454,7 +1455,8 @@ static int ext4_xattr_inode_lookup_create(handle_t *handle, struct inode *inode,
 
 static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 				struct ext4_xattr_search *s,
-				handle_t *handle, struct inode *inode)
+				handle_t *handle, struct inode *inode,
+				bool is_block)
 {
 	struct ext4_xattr_entry *last;
 	struct ext4_xattr_entry *here = s->here;
@@ -1518,8 +1520,8 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		 * attribute block so that a long value does not occupy the
 		 * whole space and prevent futher entries being added.
 		 */
-		if (ext4_has_feature_ea_inode(inode->i_sb) && new_size &&
-		    (s->end - s->base) == i_blocksize(inode) &&
+		if (ext4_has_feature_ea_inode(inode->i_sb) &&
+		    new_size && is_block &&
 		    (min_offs + old_size - new_size) <
 					EXT4_XATTR_BLOCK_RESERVE(inode)) {
 			ret = -ENOSPC;
@@ -1649,6 +1651,13 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		}
 		here->e_value_size = cpu_to_le32(i->value_len);
 	}
+
+	if (is_block) {
+		if (i->value)
+			ext4_xattr_hash_entry(here, s->base);
+		ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
+	}
+
 	ret = 0;
 out:
 	iput(old_ea_inode);
@@ -1738,14 +1747,11 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 			mb_cache_entry_delete(ea_block_cache, hash,
 					      bs->bh->b_blocknr);
 			ea_bdebug(bs->bh, "modifying in-place");
-			error = ext4_xattr_set_entry(i, s, handle, inode);
-			if (!error) {
-				if (!IS_LAST_ENTRY(s->first))
-					ext4_xattr_rehash(header(s->base),
-							  s->here);
+			error = ext4_xattr_set_entry(i, s, handle, inode,
+						     true /* is_block */);
+			if (!error)
 				ext4_xattr_block_cache_insert(ea_block_cache,
 							      bs->bh);
-			}
 			ext4_xattr_block_csum_set(inode, bs->bh);
 			unlock_buffer(bs->bh);
 			if (error == -EFSCORRUPTED)
@@ -1805,7 +1811,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, handle, inode);
+	error = ext4_xattr_set_entry(i, s, handle, inode, true /* is_block */);
 	if (error == -EFSCORRUPTED)
 		goto bad_block;
 	if (error)
@@ -1828,9 +1834,6 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 		}
 	}
 
-	if (!IS_LAST_ENTRY(s->first))
-		ext4_xattr_rehash(header(s->base), s->here);
-
 inserted:
 	if (!IS_LAST_ENTRY(s->first)) {
 		new_bh = ext4_xattr_block_cache_find(inode, header(s->base),
@@ -2063,7 +2066,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, handle, inode);
+	error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
 	if (error) {
 		if (error == -ENOSPC &&
 		    ext4_has_inline_data(inode)) {
@@ -2075,7 +2078,8 @@ 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, handle, inode);
+			error = ext4_xattr_set_entry(i, s, handle, inode,
+						     false /* is_block */);
 		}
 		if (error)
 			return error;
@@ -2101,7 +2105,7 @@ static int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
 
 	if (EXT4_I(inode)->i_extra_isize == 0)
 		return -ENOSPC;
-	error = ext4_xattr_set_entry(i, s, handle, inode);
+	error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
 	if (error)
 		return error;
 	header = IHDR(inode, ext4_raw_inode(&is->iloc));
@@ -2927,8 +2931,8 @@ ext4_xattr_block_cache_find(struct inode *inode,
  *
  * Compute the hash of an extended attribute.
  */
-static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
-					 struct ext4_xattr_entry *entry)
+static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
+				  void *value_base)
 {
 	__u32 hash = 0;
 	char *name = entry->e_name;
@@ -2941,7 +2945,7 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
 	}
 
 	if (!entry->e_value_inum && entry->e_value_size) {
-		__le32 *value = (__le32 *)((char *)header +
+		__le32 *value = (__le32 *)((char *)value_base +
 			le16_to_cpu(entry->e_value_offs));
 		for (n = (le32_to_cpu(entry->e_value_size) +
 		     EXT4_XATTR_ROUND) >> EXT4_XATTR_PAD_BITS; n; n--) {
@@ -2963,13 +2967,11 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
  *
  * Re-compute the extended attribute hash value after an entry has changed.
  */
-static void ext4_xattr_rehash(struct ext4_xattr_header *header,
-			      struct ext4_xattr_entry *entry)
+static void ext4_xattr_rehash(struct ext4_xattr_header *header)
 {
 	struct ext4_xattr_entry *here;
 	__u32 hash = 0;
 
-	ext4_xattr_hash_entry(header, entry);
 	here = ENTRY(header+1);
 	while (!IS_LAST_ENTRY(here)) {
 		if (!here->e_hash) {
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [prev] | [next] | [standalone]

#1672774 — Re: [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation for removes

On Wed, Jun 21, 2017 at 06:49:37PM -0700, Tahsin Erdogan wrote:
> When an extended attribute block is modified, ext4_xattr_hash_entry()
> recalculates e_hash for the entry that is pointed by s->here. This is
> unnecessary if the modification is to remove an entry.
> 
> Currently, if the removed entry is the last one and there are other
> entries remaining, hash calculation targets the just erased entry which
> has been filled with zeroes and effectively does nothing. If the removed
> entry is not the last one and there are more entries, this time it will
> recalculate hash on the next entry which is totally unnecessary.
> 
> Fix these by moving the decision on when to recalculate hash to
> ext4_xattr_set_entry().
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Thanks, applied.

					- Ted

[toc] | [prev] | [next] | [standalone]

#1672244 — [PATCH 31/32] ext4: strong binding of xattr inode references

To verify that a xattr entry is not pointing to the wrong xattr inode,
we currently check that the target inode has EXT4_EA_INODE_FL flag set and
also the entry size matches the target inode size.

For stronger validation, also incorporate crc32c hash of the value into
the e_hash field. This is done regardless of whether the entry lives in
the inode body or external attribute block.

Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v2: naming updates to adapt to other patch changes in the series

 fs/ext4/xattr.c | 104 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 65 insertions(+), 39 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 9cb15f2069bd..3f16dc979012 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -77,8 +77,8 @@ static void ext4_xattr_block_cache_insert(struct mb_cache *,
 static struct buffer_head *
 ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
 			    struct mb_cache_entry **);
-static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
-				  void *value_base);
+static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
+				    size_t value_count);
 static void ext4_xattr_rehash(struct ext4_xattr_header *);
 
 static const struct xattr_handler * const ext4_xattr_handler_map[] = {
@@ -380,7 +380,9 @@ static int ext4_xattr_inode_iget(struct inode *parent, unsigned long ea_ino,
 }
 
 static int
-ext4_xattr_inode_verify_hash(struct inode *ea_inode, void *buffer, size_t size)
+ext4_xattr_inode_verify_hashes(struct inode *ea_inode,
+			       struct ext4_xattr_entry *entry, void *buffer,
+			       size_t size)
 {
 	u32 hash;
 
@@ -388,23 +390,35 @@ ext4_xattr_inode_verify_hash(struct inode *ea_inode, void *buffer, size_t size)
 	hash = ext4_xattr_inode_hash(EXT4_SB(ea_inode->i_sb), buffer, size);
 	if (hash != ext4_xattr_inode_get_hash(ea_inode))
 		return -EFSCORRUPTED;
+
+	if (entry) {
+		__le32 e_hash, tmp_data;
+
+		/* Verify entry hash. */
+		tmp_data = cpu_to_le32(hash);
+		e_hash = ext4_xattr_hash_entry(entry->e_name, entry->e_name_len,
+					       &tmp_data, 1);
+		if (e_hash != entry->e_hash)
+			return -EFSCORRUPTED;
+	}
 	return 0;
 }
 
 #define EXT4_XATTR_INODE_GET_PARENT(inode) ((__u32)(inode)->i_mtime.tv_sec)
 
 /*
- * Read the value from the EA inode.
+ * Read xattr value from the EA inode.
  */
 static int
-ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
-		     size_t size)
+ext4_xattr_inode_get(struct inode *inode, struct ext4_xattr_entry *entry,
+		     void *buffer, size_t size)
 {
 	struct mb_cache *ea_inode_cache = EA_INODE_CACHE(inode);
 	struct inode *ea_inode;
 	int err;
 
-	err = ext4_xattr_inode_iget(inode, ea_ino, &ea_inode);
+	err = ext4_xattr_inode_iget(inode, le32_to_cpu(entry->e_value_inum),
+				    &ea_inode);
 	if (err) {
 		ea_inode = NULL;
 		goto out;
@@ -422,7 +436,7 @@ ext4_xattr_inode_get(struct inode *inode, unsigned long ea_ino, void *buffer,
 	if (err)
 		goto out;
 
-	err = ext4_xattr_inode_verify_hash(ea_inode, buffer, size);
+	err = ext4_xattr_inode_verify_hashes(ea_inode, entry, buffer, size);
 	/*
 	 * Compatibility check for old Lustre ea_inode implementation. Old
 	 * version does not have hash validation, but it has a backpointer
@@ -489,9 +503,8 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
 		if (size > buffer_size)
 			goto cleanup;
 		if (entry->e_value_inum) {
-			error = ext4_xattr_inode_get(inode,
-					     le32_to_cpu(entry->e_value_inum),
-					     buffer, size);
+			error = ext4_xattr_inode_get(inode, entry, buffer,
+						     size);
 			if (error)
 				goto cleanup;
 		} else {
@@ -539,9 +552,8 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 		if (size > buffer_size)
 			goto cleanup;
 		if (entry->e_value_inum) {
-			error = ext4_xattr_inode_get(inode,
-					     le32_to_cpu(entry->e_value_inum),
-					     buffer, size);
+			error = ext4_xattr_inode_get(inode, entry, buffer,
+						     size);
 			if (error)
 				goto cleanup;
 		} else {
@@ -1387,8 +1399,8 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
 		    (EXT4_I(ea_inode)->i_flags & EXT4_EA_INODE_FL) &&
 		    i_size_read(ea_inode) == value_len &&
 		    !ext4_xattr_inode_read(ea_inode, ea_data, value_len) &&
-		    !ext4_xattr_inode_verify_hash(ea_inode, ea_data,
-						  value_len) &&
+		    !ext4_xattr_inode_verify_hashes(ea_inode, NULL, ea_data,
+						    value_len) &&
 		    !memcmp(value, ea_data, value_len)) {
 			mb_cache_entry_touch(ea_inode_cache, ce);
 			mb_cache_entry_put(ea_inode_cache, ce);
@@ -1652,12 +1664,36 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		here->e_value_size = cpu_to_le32(i->value_len);
 	}
 
-	if (is_block) {
-		if (i->value)
-			ext4_xattr_hash_entry(here, s->base);
-		ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
+	if (i->value) {
+		__le32 hash = 0;
+
+		/* Entry hash calculation. */
+		if (in_inode) {
+			__le32 crc32c_hash;
+
+			/*
+			 * Feed crc32c hash instead of the raw value for entry
+			 * hash calculation. This is to avoid walking
+			 * potentially long value buffer again.
+			 */
+			crc32c_hash = cpu_to_le32(
+				       ext4_xattr_inode_get_hash(new_ea_inode));
+			hash = ext4_xattr_hash_entry(here->e_name,
+						     here->e_name_len,
+						     &crc32c_hash, 1);
+		} else if (is_block) {
+			__le32 *value = s->base + min_offs - new_size;
+
+			hash = ext4_xattr_hash_entry(here->e_name,
+						     here->e_name_len, value,
+						     new_size >> 2);
+		}
+		here->e_hash = hash;
 	}
 
+	if (is_block)
+		ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
+
 	ret = 0;
 out:
 	iput(old_ea_inode);
@@ -2439,9 +2475,7 @@ static int ext4_xattr_move_to_block(handle_t *handle, struct inode *inode,
 
 	/* Save the entry name and the entry value */
 	if (entry->e_value_inum) {
-		error = ext4_xattr_inode_get(inode,
-					     le32_to_cpu(entry->e_value_inum),
-					     buffer, value_size);
+		error = ext4_xattr_inode_get(inode, entry, buffer, value_size);
 		if (error)
 			goto out;
 	} else {
@@ -2931,30 +2965,22 @@ ext4_xattr_block_cache_find(struct inode *inode,
  *
  * Compute the hash of an extended attribute.
  */
-static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
-				  void *value_base)
+static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
+				    size_t value_count)
 {
 	__u32 hash = 0;
-	char *name = entry->e_name;
-	int n;
 
-	for (n = 0; n < entry->e_name_len; n++) {
+	while (name_len--) {
 		hash = (hash << NAME_HASH_SHIFT) ^
 		       (hash >> (8*sizeof(hash) - NAME_HASH_SHIFT)) ^
 		       *name++;
 	}
-
-	if (!entry->e_value_inum && entry->e_value_size) {
-		__le32 *value = (__le32 *)((char *)value_base +
-			le16_to_cpu(entry->e_value_offs));
-		for (n = (le32_to_cpu(entry->e_value_size) +
-		     EXT4_XATTR_ROUND) >> EXT4_XATTR_PAD_BITS; n; n--) {
-			hash = (hash << VALUE_HASH_SHIFT) ^
-			       (hash >> (8*sizeof(hash) - VALUE_HASH_SHIFT)) ^
-			       le32_to_cpu(*value++);
-		}
+	while (value_count--) {
+		hash = (hash << VALUE_HASH_SHIFT) ^
+		       (hash >> (8*sizeof(hash) - VALUE_HASH_SHIFT)) ^
+		       le32_to_cpu(*value++);
 	}
-	entry->e_hash = cpu_to_le32(hash);
+	return cpu_to_le32(hash);
 }
 
 #undef NAME_HASH_SHIFT
-- 
2.13.1.611.g7e3b11ae1-goog

[toc] | [prev] | [next] | [standalone]

#1672778 — Re: [PATCH 31/32] ext4: strong binding of xattr inode references

On Wed, Jun 21, 2017 at 06:49:38PM -0700, Tahsin Erdogan wrote:
> To verify that a xattr entry is not pointing to the wrong xattr inode,
> we currently check that the target inode has EXT4_EA_INODE_FL flag set and
> also the entry size matches the target inode size.
> 
> For stronger validation, also incorporate crc32c hash of the value into
> the e_hash field. This is done regardless of whether the entry lives in
> the inode body or external attribute block.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Added to the ext4 patch queue, thanks.

					- Ted

[toc] | [prev] | [next] | [standalone]

#1672754

On Wed, Jun 21, 2017 at 06:49:31PM -0700, Tahsin Erdogan wrote:
> There will be a second mb_cache instance that tracks ea_inodes. Make
> existing names more explicit so that it is clear that they refer to
> xattr block cache.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Thanks, applied.

					- Ted

[toc] | [prev] | [next] | [standalone]

#1672769 — Re: [PATCH 27/32] ext4: xattr inode deduplication

On Wed, Jun 21, 2017 at 06:49:34PM -0700, Tahsin Erdogan wrote:
> Ext4 now supports xattr values that are up to 64k in size (vfs limit).
> Large xattr values are stored in external inodes each one holding a
> single value. Once written the data blocks of these inodes are immutable.
> 
> The real world use cases are expected to have a lot of value duplication
> such as inherited acls etc. To reduce data duplication on disk, this patch
> implements a deduplicator that allows sharing of xattr inodes.
> 
> The deduplication is based on an in-memory hash lookup that is a best
> effort sharing scheme. When a xattr inode is read from disk (i.e.
> getxattr() call), its crc32c hash is added to a hash table. Before
> creating a new xattr inode for a value being set, the hash table is
> checked to see if an existing inode holds an identical value. If such an
> inode is found, the ref count on that inode is incremented. On value
> removal the ref count is decremented and if it reaches zero the inode is
> deleted.
> 
> The quota charging for such inodes is manually managed. Every reference
> holder is charged the full size as if there was no sharing happening.
> This is consistent with how xattr blocks are also charged.
> 
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Applied, thanks

				- Ted

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web