linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tahsin Erdogan <tahsin@google.com>
To: Andreas Dilger <adilger@dilger.ca>,
	"Darrick J . Wong" <darrick.wong@oracle.com>,
	Jan Kara <jack@suse.cz>, Theodore Ts'o <tytso@mit.edu>,
	linux-ext4@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Tahsin Erdogan <tahsin@google.com>
Subject: [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation for removes
Date: Wed, 21 Jun 2017 14:21:40 -0700	[thread overview]
Message-ID: <20170621212142.16581-30-tahsin@google.com> (raw)
In-Reply-To: <20170621212142.16581-1-tahsin@google.com>

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 e5ff4d5a0618..b1bd60244978 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(mb_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(mb_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

  parent reply	other threads:[~2017-06-21 21:21 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-21 21:21 [PATCH 01/32] ext4: xattr-in-inode support Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 02/32] ext4: fix lockdep warning about recursive inode locking Tahsin Erdogan
2017-06-22  1:18   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 03/32] ext4: lock inode before calling ext4_orphan_add() Tahsin Erdogan
2017-06-22  1:20   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 04/32] ext4: do not set posix acls on xattr inodes Tahsin Erdogan
2017-06-22  1:23   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 05/32] ext4: attach jinode after creation of xattr inode Tahsin Erdogan
2017-06-22  1:26   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 06/32] ext4: ea_inode owner should be the same as the inode owner Tahsin Erdogan
2017-06-22  1:28   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 07/32] ext4: call journal revoke when freeing ea_inode blocks Tahsin Erdogan
2017-06-22  1:38   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 08/32] ext4: fix ref counting for ea_inode Tahsin Erdogan
2017-06-22  1:40   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 09/32] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan
2017-06-22  1:42   ` Theodore Ts'o
2017-06-22 18:02   ` Darrick J. Wong
2017-06-22 19:40     ` Andreas Dilger
2017-06-22 22:27       ` Theodore Ts'o
2017-06-22 23:21         ` Darrick J. Wong
2017-06-21 21:21 ` [PATCH 10/32] ext4: change ext4_xattr_inode_iget() signature Tahsin Erdogan
2017-06-22  1:55   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 11/32] ext4: clean up ext4_xattr_inode_get() Tahsin Erdogan
2017-06-22  1:58   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 12/32] ext4: add missing le32_to_cpu(e_value_inum) conversions Tahsin Erdogan
2017-06-22  2:00   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 13/32] ext4: ext4_xattr_value_same() should return false for external data Tahsin Erdogan
2017-06-22  2:04   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 14/32] ext4: fix ext4_xattr_make_inode_space() value size calculation Tahsin Erdogan
2017-06-22  2:09   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 15/32] ext4: fix ext4_xattr_move_to_block() Tahsin Erdogan
2017-06-22  2:13   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 16/32] ext4: fix ext4_xattr_cmp() Tahsin Erdogan
2017-06-22  2:15   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 17/32] ext4: fix credits calculation for xattr inode Tahsin Erdogan
2017-06-22  2:19   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 18/32] ext4: retry storing value in external inode with xattr block too Tahsin Erdogan
2017-06-22  2:22   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 19/32] ext4: ext4_xattr_delete_inode() should return accurate errors Tahsin Erdogan
2017-06-22  2:27   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 20/32] ext4: improve journal credit handling in set xattr paths Tahsin Erdogan
2017-06-22  2:30   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 21/32] ext4: modify ext4_xattr_ino_array to hold struct inode * Tahsin Erdogan
2017-06-22 14:28   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 22/32] ext4: move struct ext4_xattr_inode_array to xattr.h Tahsin Erdogan
2017-06-22 14:29   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 23/32] mbcache: make mbcache naming more generic Tahsin Erdogan
2017-06-22 14:37   ` Theodore Ts'o
2017-06-21 21:21 ` [PATCH 24/32] ext2, ext4: make mb block cache names more explicit Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 25/32] ext4: add ext4_is_quota_file() Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 26/32] ext4: cleanup transaction restarts during inode deletion Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 27/32] ext4: xattr inode deduplication Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 28/32] quota: add get_inode_usage callback to transfer multi-inode charges Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 29/32] ext4: reserve space for xattr entries/names Tahsin Erdogan
2017-06-21 21:21 ` Tahsin Erdogan [this message]
2017-06-21 21:21 ` [PATCH 31/32] ext4: strong binding of xattr inode references Tahsin Erdogan
2017-06-21 21:21 ` [PATCH 32/32] ext4: add nombcache mount option Tahsin Erdogan
2017-06-22  1:16 ` [PATCH 01/32] ext4: xattr-in-inode support Theodore Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2017-06-22  1:49 [PATCH 24/32] ext2, ext4: make mb block cache names more explicit Tahsin Erdogan
2017-06-22  1:49 ` [PATCH 30/32] ext4: eliminate xattr entry e_hash recalculation for removes Tahsin Erdogan
2017-06-22 15:52   ` Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170621212142.16581-30-tahsin@google.com \
    --to=tahsin@google.com \
    --cc=adilger@dilger.ca \
    --cc=darrick.wong@oracle.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).