From 554cf5b70ebe73569c72ccc1fd8a0e1357143cbc Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 22 Jun 2017 11:52:19 -0400 Subject: [PATCH] add patch eliminate-xattr-entry-e_hash-recalcuation-for-removes --- ...ate-xattr-entry-e_hash-recalcuation-for-removes | 178 +++++++++++++++++++++ series | 1 + timestamps | 7 +- 3 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 eliminate-xattr-entry-e_hash-recalcuation-for-removes diff --git a/eliminate-xattr-entry-e_hash-recalcuation-for-removes b/eliminate-xattr-entry-e_hash-recalcuation-for-removes new file mode 100644 index 00000000..61212990 --- /dev/null +++ b/eliminate-xattr-entry-e_hash-recalcuation-for-removes @@ -0,0 +1,178 @@ +ext4: eliminate xattr entry e_hash recalculation for removes + +From: Tahsin Erdogan + +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 +Signed-off-by: Theodore Ts'o +--- + 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 + + diff --git a/series b/series index 515f30be..ab5e2583 100644 --- a/series +++ b/series @@ -30,6 +30,7 @@ cleanup-transaction-restarts-during-inode-deletion xattr-inode-deduplication add-get_inode_usage-callback-to-transfer-multi-inode-charges reserve-space-for-xattr-entries_names +eliminate-xattr-entry-e_hash-recalcuation-for-removes #################################################### # unstable patches diff --git a/timestamps b/timestamps index 7856f6d0..175c87fb 100755 --- a/timestamps +++ b/timestamps @@ -62,7 +62,8 @@ touch -d @1498145485 add-ext4_is_quota_file touch -d @1498146129 cleanup-transaction-restarts-during-inode-deletion touch -d @1498146295 xattr-inode-deduplication touch -d @1498146408 add-get_inode_usage-callback-to-transfer-multi-inode-charges -touch -d @1498146424 timestamps touch -d @1498146533 reserve-space-for-xattr-entries_names -touch -d @1498146559 series -touch -d @1498146562 status +touch -d @1498146710 series +touch -d @1498146723 eliminate-xattr-entry-e_hash-recalcuation-for-removes +touch -d @1498146730 status +touch -d @1498146731 timestamps -- 2.11.4.GIT