eliminate-xattr-entry-e_hash-recalcuation-for-removes

   1 ext4: eliminate xattr entry e_hash recalculation for removes
   2
   3 From: Tahsin Erdogan <tahsin@google.com>
   4
   5 When an extended attribute block is modified, ext4_xattr_hash_entry()
   6 recalculates e_hash for the entry that is pointed by s->here. This  is
   7 unnecessary if the modification is to remove an entry.
   8
   9 Currently, if the removed entry is the last one and there are other
  10 entries remaining, hash calculation targets the just erased entry which
  11 has been filled with zeroes and effectively does nothing.  If the removed
  12 entry is not the last one and there are more entries, this time it will
  13 recalculate hash on the next entry which is totally unnecessary.
  14
  15 Fix these by moving the decision on when to recalculate hash to
  16 ext4_xattr_set_entry().
  17
  18 Signed-off-by: Tahsin Erdogan <tahsin@google.com>
  19 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  20 ---
  21  fs/ext4/xattr.c | 50 ++++++++++++++++++++++++++------------------------
  22  1 file changed, 26 insertions(+), 24 deletions(-)
  23
  24 diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
  25 index 835b3dca5b65..9cb15f2069bd 100644
  26 --- a/fs/ext4/xattr.c
  27 +++ b/fs/ext4/xattr.c
  28 @@ -77,8 +77,9 @@ static void ext4_xattr_block_cache_insert(struct mb_cache *,
  29  static struct buffer_head *
  30  ext4_xattr_block_cache_find(struct inode *, struct ext4_xattr_header *,
  31                             struct mb_cache_entry **);
  32 -static void ext4_xattr_rehash(struct ext4_xattr_header *,
  33 -                             struct ext4_xattr_entry *);
  34 +static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
  35 +                                 void *value_base);
  36 +static void ext4_xattr_rehash(struct ext4_xattr_header *);
  37
  38  static const struct xattr_handler * const ext4_xattr_handler_map[] = {
  39         [EXT4_XATTR_INDEX_USER]              = &ext4_xattr_user_handler,
  40 @@ -1454,7 +1455,8 @@ static int ext4_xattr_inode_lookup_create(handle_t *handle, struct inode *inode,
  41
  42  static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
  43                                 struct ext4_xattr_search *s,
  44 -                               handle_t *handle, struct inode *inode)
  45 +                               handle_t *handle, struct inode *inode,
  46 +                               bool is_block)
  47  {
  48         struct ext4_xattr_entry *last;
  49         struct ext4_xattr_entry *here = s->here;
  50 @@ -1518,8 +1520,8 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
  51                  * attribute block so that a long value does not occupy the
  52                  * whole space and prevent futher entries being added.
  53                  */
  54 -               if (ext4_has_feature_ea_inode(inode->i_sb) && new_size &&
  55 -                   (s->end - s->base) == i_blocksize(inode) &&
  56 +               if (ext4_has_feature_ea_inode(inode->i_sb) &&
  57 +                   new_size && is_block &&
  58                     (min_offs + old_size - new_size) <
  59                                         EXT4_XATTR_BLOCK_RESERVE(inode)) {
  60                         ret = -ENOSPC;
  61 @@ -1649,6 +1651,13 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
  62                 }
  63                 here->e_value_size = cpu_to_le32(i->value_len);
  64         }
  65 +
  66 +       if (is_block) {
  67 +               if (i->value)
  68 +                       ext4_xattr_hash_entry(here, s->base);
  69 +               ext4_xattr_rehash((struct ext4_xattr_header *)s->base);
  70 +       }
  71 +
  72         ret = 0;
  73  out:
  74         iput(old_ea_inode);
  75 @@ -1738,14 +1747,11 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
  76                         mb_cache_entry_delete(ea_block_cache, hash,
  77                                               bs->bh->b_blocknr);
  78                         ea_bdebug(bs->bh, "modifying in-place");
  79 -                       error = ext4_xattr_set_entry(i, s, handle, inode);
  80 -                       if (!error) {
  81 -                               if (!IS_LAST_ENTRY(s->first))
  82 -                                       ext4_xattr_rehash(header(s->base),
  83 -                                                         s->here);
  84 +                       error = ext4_xattr_set_entry(i, s, handle, inode,
  85 +                                                    true /* is_block */);
  86 +                       if (!error)
  87                                 ext4_xattr_block_cache_insert(ea_block_cache,
  88                                                               bs->bh);
  89 -                       }
  90                         ext4_xattr_block_csum_set(inode, bs->bh);
  91                         unlock_buffer(bs->bh);
  92                         if (error == -EFSCORRUPTED)
  93 @@ -1805,7 +1811,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
  94                 s->end = s->base + sb->s_blocksize;
  95         }
  96
  97 -       error = ext4_xattr_set_entry(i, s, handle, inode);
  98 +       error = ext4_xattr_set_entry(i, s, handle, inode, true /* is_block */);
  99         if (error == -EFSCORRUPTED)
 100                 goto bad_block;
 101         if (error)
 102 @@ -1828,9 +1834,6 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 103                 }
 104         }
 105
 106 -       if (!IS_LAST_ENTRY(s->first))
 107 -               ext4_xattr_rehash(header(s->base), s->here);
 108 -
 109  inserted:
 110         if (!IS_LAST_ENTRY(s->first)) {
 111                 new_bh = ext4_xattr_block_cache_find(inode, header(s->base),
 112 @@ -2063,7 +2066,7 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 113
 114         if (EXT4_I(inode)->i_extra_isize == 0)
 115                 return -ENOSPC;
 116 -       error = ext4_xattr_set_entry(i, s, handle, inode);
 117 +       error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
 118         if (error) {
 119                 if (error == -ENOSPC &&
 120                     ext4_has_inline_data(inode)) {
 121 @@ -2075,7 +2078,8 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
 122                         error = ext4_xattr_ibody_find(inode, i, is);
 123                         if (error)
 124                                 return error;
 125 -                       error = ext4_xattr_set_entry(i, s, handle, inode);
 126 +                       error = ext4_xattr_set_entry(i, s, handle, inode,
 127 +                                                    false /* is_block */);
 128                 }
 129                 if (error)
 130                         return error;
 131 @@ -2101,7 +2105,7 @@ static int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
 132
 133         if (EXT4_I(inode)->i_extra_isize == 0)
 134                 return -ENOSPC;
 135 -       error = ext4_xattr_set_entry(i, s, handle, inode);
 136 +       error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
 137         if (error)
 138                 return error;
 139         header = IHDR(inode, ext4_raw_inode(&is->iloc));
 140 @@ -2927,8 +2931,8 @@ ext4_xattr_block_cache_find(struct inode *inode,
 141   *
 142   * Compute the hash of an extended attribute.
 143   */
 144 -static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
 145 -                                        struct ext4_xattr_entry *entry)
 146 +static void ext4_xattr_hash_entry(struct ext4_xattr_entry *entry,
 147 +                                 void *value_base)
 148  {
 149         __u32 hash = 0;
 150         char *name = entry->e_name;
 151 @@ -2941,7 +2945,7 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
 152         }
 153
 154         if (!entry->e_value_inum && entry->e_value_size) {
 155 -               __le32 *value = (__le32 *)((char *)header +
 156 +               __le32 *value = (__le32 *)((char *)value_base +
 157                         le16_to_cpu(entry->e_value_offs));
 158                 for (n = (le32_to_cpu(entry->e_value_size) +
 159                      EXT4_XATTR_ROUND) >> EXT4_XATTR_PAD_BITS; n; n--) {
 160 @@ -2963,13 +2967,11 @@ static inline void ext4_xattr_hash_entry(struct ext4_xattr_header *header,
 161   *
 162   * Re-compute the extended attribute hash value after an entry has changed.
 163   */
 164 -static void ext4_xattr_rehash(struct ext4_xattr_header *header,
 165 -                             struct ext4_xattr_entry *entry)
 166 +static void ext4_xattr_rehash(struct ext4_xattr_header *header)
 167  {
 168         struct ext4_xattr_entry *here;
 169         __u32 hash = 0;
 170
 171 -       ext4_xattr_hash_entry(header, entry);
 172         here = ENTRY(header+1);
 173         while (!IS_LAST_ENTRY(here)) {
 174                 if (!here->e_hash) {
 175 --
 176 2.13.1.611.g7e3b11ae1-goog
 177
 178