From: Andreas Dilger <adilger@dilger.ca>
To: Tahsin Erdogan <tahsin@google.com>
Cc: Jan Kara <jack@suse.cz>,
linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 23/31] mbcache: make mbcache naming more generic
Date: Wed, 21 Jun 2017 11:43:18 -0600 [thread overview]
Message-ID: <3DC421C9-456D-4A96-AE38-2F8516F62B91@dilger.ca> (raw)
In-Reply-To: <20170620090113.6658-1-tahsin@google.com>
[-- Attachment #1: Type: text/plain, Size: 11405 bytes --]
On Jun 20, 2017, at 3:01 AM, Tahsin Erdogan <tahsin@google.com> wrote:
>
> Make names more generic so that mbcache usage is not limited to
> block sharing. In a subsequent patch in the series
> ("ext4: xattr inode deduplication"), we start using the mbcache code
> for sharing xattr inodes. With that patch, old mb_cache_entry.e_block
> field could be holding either a block number or an inode number.
>
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
> v2: updated commit title and description
>
> fs/ext2/xattr.c | 18 +++++++++---------
> fs/ext4/xattr.c | 10 +++++-----
> fs/mbcache.c | 43 +++++++++++++++++++++----------------------
> include/linux/mbcache.h | 11 +++++------
> 4 files changed, 40 insertions(+), 42 deletions(-)
>
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index fbdb8f171893..1e5f76070580 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -493,8 +493,8 @@ 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_block(EXT2_SB(sb)->s_mb_cache,
> - hash, bh->b_blocknr);
> + mb_cache_entry_delete(EXT2_SB(sb)->s_mb_cache, hash,
> + bh->b_blocknr);
>
> /* keep the buffer locked while modifying it. */
> } else {
> @@ -721,8 +721,8 @@ 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_block(ext2_mb_cache,
> - hash, old_bh->b_blocknr);
> + mb_cache_entry_delete(ext2_mb_cache, hash,
> + old_bh->b_blocknr);
> /* Free the old block. */
> ea_bdebug(old_bh, "freeing");
> ext2_free_blocks(inode, old_bh->b_blocknr, 1);
> @@ -795,8 +795,8 @@ 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_block(EXT2_SB(inode->i_sb)->s_mb_cache,
> - hash, bh->b_blocknr);
> + mb_cache_entry_delete(EXT2_SB(inode->i_sb)->s_mb_cache, hash,
> + bh->b_blocknr);
> ext2_free_blocks(inode, EXT2_I(inode)->i_file_acl, 1);
> get_bh(bh);
> bforget(bh);
> @@ -907,11 +907,11 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
> while (ce) {
> struct buffer_head *bh;
>
> - bh = sb_bread(inode->i_sb, ce->e_block);
> + bh = sb_bread(inode->i_sb, ce->e_value);
> if (!bh) {
> ext2_error(inode->i_sb, "ext2_xattr_cache_find",
> "inode %ld: block %ld read error",
> - inode->i_ino, (unsigned long) ce->e_block);
> + inode->i_ino, (unsigned long) ce->e_value);
> } else {
> lock_buffer(bh);
> /*
> @@ -931,7 +931,7 @@ ext2_xattr_cache_find(struct inode *inode, struct ext2_xattr_header *header)
> } else if (le32_to_cpu(HDR(bh)->h_refcount) >
> EXT2_XATTR_REFCOUNT_MAX) {
> ea_idebug(inode, "block %ld refcount %d>%d",
> - (unsigned long) ce->e_block,
> + (unsigned long) ce->e_value,
> le32_to_cpu(HDR(bh)->h_refcount),
> EXT2_XATTR_REFCOUNT_MAX);
> } else if (!ext2_xattr_cmp(header, HDR(bh))) {
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index c09fcffb0878..0b43e0e52e26 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -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_block(ext4_mb_cache, hash, bh->b_blocknr);
> + mb_cache_entry_delete(ext4_mb_cache, hash, bh->b_blocknr);
> get_bh(bh);
> unlock_buffer(bh);
> ext4_free_blocks(handle, inode, bh, 0, 1,
> @@ -1115,8 +1115,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
> * ext4_xattr_block_set() to reliably detect modified
> * block
> */
> - mb_cache_entry_delete_block(ext4_mb_cache, hash,
> - bs->bh->b_blocknr);
> + mb_cache_entry_delete(ext4_mb_cache, hash,
> + bs->bh->b_blocknr);
> ea_bdebug(bs->bh, "modifying in-place");
> error = ext4_xattr_set_entry(i, s, handle, inode);
> if (!error) {
> @@ -2238,10 +2238,10 @@ ext4_xattr_cache_find(struct inode *inode, struct ext4_xattr_header *header,
> while (ce) {
> struct buffer_head *bh;
>
> - bh = sb_bread(inode->i_sb, ce->e_block);
> + bh = sb_bread(inode->i_sb, ce->e_value);
> if (!bh) {
> EXT4_ERROR_INODE(inode, "block %lu read error",
> - (unsigned long) ce->e_block);
> + (unsigned long) ce->e_value);
> } else if (ext4_xattr_cmp(header, BHDR(bh)) == 0) {
> *pce = ce;
> return bh;
> diff --git a/fs/mbcache.c b/fs/mbcache.c
> index b19be429d655..45a8d52dc991 100644
> --- a/fs/mbcache.c
> +++ b/fs/mbcache.c
> @@ -10,7 +10,7 @@
> /*
> * Mbcache is a simple key-value store. Keys need not be unique, however
> * key-value pairs are expected to be unique (we use this fact in
> - * mb_cache_entry_delete_block()).
> + * mb_cache_entry_delete()).
> *
> * Ext2 and ext4 use this cache for deduplication of extended attribute blocks.
> * They use hash of a block contents as a key and block number as a value.
> @@ -62,15 +62,15 @@ static inline struct hlist_bl_head *mb_cache_entry_head(struct mb_cache *cache,
> * @cache - cache where the entry should be created
> * @mask - gfp mask with which the entry should be allocated
> * @key - key of the entry
> - * @block - block that contains data
> - * @reusable - is the block reusable by other inodes?
> + * @value - value of the entry
> + * @reusable - is the entry reusable by others?
> *
> - * Creates entry in @cache with key @key and records that data is stored in
> - * block @block. The function returns -EBUSY if entry with the same key
> - * and for the same block already exists in cache. Otherwise 0 is returned.
> + * Creates entry in @cache with key @key and value @value. The function returns
> + * -EBUSY if entry with the same key and value already exists in cache.
> + * Otherwise 0 is returned.
> */
> int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
> - sector_t block, bool reusable)
> + u64 value, bool reusable)
> {
> struct mb_cache_entry *entry, *dup;
> struct hlist_bl_node *dup_node;
> @@ -91,12 +91,12 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
> /* One ref for hash, one ref returned */
> atomic_set(&entry->e_refcnt, 1);
> entry->e_key = key;
> - entry->e_block = block;
> + entry->e_value = value;
> entry->e_reusable = reusable;
> head = mb_cache_entry_head(cache, key);
> hlist_bl_lock(head);
> hlist_bl_for_each_entry(dup, dup_node, head, e_hash_list) {
> - if (dup->e_key == key && dup->e_block == block) {
> + if (dup->e_key == key && dup->e_value == value) {
> hlist_bl_unlock(head);
> kmem_cache_free(mb_entry_cache, entry);
> return -EBUSY;
> @@ -187,13 +187,13 @@ struct mb_cache_entry *mb_cache_entry_find_next(struct mb_cache *cache,
> EXPORT_SYMBOL(mb_cache_entry_find_next);
>
> /*
> - * mb_cache_entry_get - get a cache entry by block number (and key)
> + * mb_cache_entry_get - get a cache entry by value (and key)
> * @cache - cache we work with
> - * @key - key of block number @block
> - * @block - block number
> + * @key - key
> + * @value - value
> */
> struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
> - sector_t block)
> + u64 value)
> {
> struct hlist_bl_node *node;
> struct hlist_bl_head *head;
> @@ -202,7 +202,7 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
> head = mb_cache_entry_head(cache, key);
> hlist_bl_lock(head);
> hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
> - if (entry->e_key == key && entry->e_block == block) {
> + if (entry->e_key == key && entry->e_value == value) {
> atomic_inc(&entry->e_refcnt);
> goto out;
> }
> @@ -214,15 +214,14 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
> }
> EXPORT_SYMBOL(mb_cache_entry_get);
>
> -/* mb_cache_entry_delete_block - remove information about block from cache
> +/* mb_cache_entry_delete - remove a cache entry
> * @cache - cache we work with
> - * @key - key of block @block
> - * @block - block number
> + * @key - key
> + * @value - value
> *
> - * Remove entry from cache @cache with key @key with data stored in @block.
> + * Remove entry from cache @cache with key @key and value @value.
> */
> -void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
> - sector_t block)
> +void mb_cache_entry_delete(struct mb_cache *cache, u32 key, u64 value)
> {
> struct hlist_bl_node *node;
> struct hlist_bl_head *head;
> @@ -231,7 +230,7 @@ void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
> head = mb_cache_entry_head(cache, key);
> hlist_bl_lock(head);
> hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
> - if (entry->e_key == key && entry->e_block == block) {
> + if (entry->e_key == key && entry->e_value == value) {
> /* We keep hash list reference to keep entry alive */
> hlist_bl_del_init(&entry->e_hash_list);
> hlist_bl_unlock(head);
> @@ -248,7 +247,7 @@ void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
> }
> hlist_bl_unlock(head);
> }
> -EXPORT_SYMBOL(mb_cache_entry_delete_block);
> +EXPORT_SYMBOL(mb_cache_entry_delete);
>
> /* mb_cache_entry_touch - cache entry got used
> * @cache - cache the entry belongs to
> diff --git a/include/linux/mbcache.h b/include/linux/mbcache.h
> index 86c9a8b480c5..e1bc73414983 100644
> --- a/include/linux/mbcache.h
> +++ b/include/linux/mbcache.h
> @@ -19,15 +19,15 @@ struct mb_cache_entry {
> u32 e_key;
> u32 e_referenced:1;
> u32 e_reusable:1;
> - /* Block number of hashed block - stable during lifetime of the entry */
> - sector_t e_block;
> + /* User provided value - stable during lifetime of the entry */
> + u64 e_value;
> };
>
> struct mb_cache *mb_cache_create(int bucket_bits);
> void mb_cache_destroy(struct mb_cache *cache);
>
> int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
> - sector_t block, bool reusable);
> + u64 value, bool reusable);
> void __mb_cache_entry_free(struct mb_cache_entry *entry);
> static inline int mb_cache_entry_put(struct mb_cache *cache,
> struct mb_cache_entry *entry)
> @@ -38,10 +38,9 @@ static inline int mb_cache_entry_put(struct mb_cache *cache,
> return 1;
> }
>
> -void mb_cache_entry_delete_block(struct mb_cache *cache, u32 key,
> - sector_t block);
> +void mb_cache_entry_delete(struct mb_cache *cache, u32 key, u64 value);
> struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
> - sector_t block);
> + u64 value);
> struct mb_cache_entry *mb_cache_entry_find_first(struct mb_cache *cache,
> u32 key);
> struct mb_cache_entry *mb_cache_entry_find_next(struct mb_cache *cache,
> --
> 2.13.1.518.g3df882009-goog
>
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
next prev parent reply other threads:[~2017-06-21 17:43 UTC|newest]
Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-31 8:14 [PATCH 01/28] ext4: xattr-in-inode support Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 02/28] ext4: fix lockdep warning about recursive inode locking Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 03/28] ext4: lock inode before calling ext4_orphan_add() Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 04/28] ext4: do not set posix acls on xattr inodes Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 05/28] ext4: attach jinode after creation of xattr inode Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 06/28] ext4: ea_inode owner should be the same as the inode owner Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 07/28] ext4: call journal revoke when freeing ea_inode blocks Tahsin Erdogan
2017-05-31 16:12 ` Darrick J. Wong
2017-05-31 16:12 ` [Ocfs2-devel] " Darrick J. Wong
2017-05-31 16:12 ` Darrick J. Wong
2017-05-31 21:01 ` Tahsin Erdogan
2017-06-05 22:08 ` Andreas Dilger
2017-05-31 8:14 ` [PATCH 08/28] ext4: fix ref counting for ea_inode Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 09/28] ext4: extended attribute value size limit is enforced by vfs Tahsin Erdogan
2017-05-31 16:03 ` Darrick J. Wong
2017-05-31 16:03 ` [Ocfs2-devel] " Darrick J. Wong
2017-05-31 16:03 ` Darrick J. Wong
2017-05-31 16:13 ` Tahsin Erdogan
2017-05-31 8:14 ` [PATCH 10/28] ext4: change ext4_xattr_inode_iget() signature Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 11/28] ext4: clean up ext4_xattr_inode_get() Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 12/28] ext4: add missing le32_to_cpu(e_value_inum) conversions Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 13/28] ext4: ext4_xattr_value_same() should return false for external data Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 14/28] ext4: fix ext4_xattr_make_inode_space() value size calculation Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 15/28] ext4: fix ext4_xattr_move_to_block() Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 16/28] ext4: fix ext4_xattr_cmp() Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 17/28] ext4: fix credits calculation for xattr inode Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 18/28] ext4: retry storing value in external inode with xattr block too Tahsin Erdogan
2017-06-20 8:56 ` [PATCH v2 18/31] " Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 19/28] ext4: ext4_xattr_delete_inode() should return accurate errors Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 20/28] ext4: improve journal credit handling in set xattr paths Tahsin Erdogan
2017-06-20 8:59 ` [PATCH v2 20/31] " Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 21/28] ext4: modify ext4_xattr_ino_array to hold struct inode * Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 22/28] ext4: move struct ext4_xattr_inode_array to xattr.h Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 23/28] mbcache: make mbcache more generic Tahsin Erdogan
2017-06-15 7:41 ` Jan Kara
2017-06-15 7:41 ` [Ocfs2-devel] " Jan Kara
2017-06-15 18:25 ` Tahsin Erdogan
2017-06-19 8:50 ` Jan Kara
2017-06-19 8:50 ` [Ocfs2-devel] " Jan Kara
2017-06-20 9:01 ` [PATCH v2 23/31] mbcache: make mbcache naming " Tahsin Erdogan
2017-06-21 17:43 ` Andreas Dilger [this message]
2017-06-21 18:33 ` Andreas Dilger
2017-06-21 21:39 ` Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 24/28] ext4: rename mb block cache functions Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 25/28] ext4: add ext4_is_quota_file() Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 26/28] ext4: cleanup transaction restarts during inode deletion Tahsin Erdogan
2017-06-14 14:17 ` [PATCH v2 " Tahsin Erdogan
2017-06-15 0:11 ` Andreas Dilger
2017-06-15 0:11 ` [Ocfs2-devel] " Andreas Dilger
2017-06-20 9:04 ` [PATCH v3 " Tahsin Erdogan
2017-06-20 9:29 ` Tahsin Erdogan
2017-05-31 8:15 ` [PATCH 27/28] ext4: xattr inode deduplication Tahsin Erdogan
2017-05-31 15:40 ` kbuild test robot
2017-05-31 15:40 ` [Ocfs2-devel] " kbuild test robot
2017-05-31 15:50 ` kbuild test robot
2017-05-31 15:50 ` [Ocfs2-devel] " kbuild test robot
2017-05-31 16:00 ` Darrick J. Wong
2017-05-31 16:00 ` [Ocfs2-devel] " Darrick J. Wong
2017-05-31 16:00 ` Darrick J. Wong
2017-05-31 22:33 ` [PATCH v2 " Tahsin Erdogan
2017-06-02 5:41 ` Darrick J. Wong
2017-06-02 5:41 ` [Ocfs2-devel] " Darrick J. Wong
2017-06-02 5:41 ` Darrick J. Wong
2017-06-02 12:46 ` Tahsin Erdogan
2017-06-02 17:59 ` Darrick J. Wong
2017-06-02 17:59 ` [Ocfs2-devel] " Darrick J. Wong
2017-06-02 17:59 ` Darrick J. Wong
2017-06-02 23:35 ` [PATCH v3 " Tahsin Erdogan
2017-06-14 14:34 ` [PATCH v4 " Tahsin Erdogan
2017-06-14 23:26 ` Andreas Dilger
2017-06-20 9:07 ` [PATCH v5 " Tahsin Erdogan
2017-06-20 9:49 ` Tahsin Erdogan
2017-06-21 17:42 ` Andreas Dilger
2017-06-21 21:14 ` Andreas Dilger
2017-06-21 21:34 ` Tahsin Erdogan
2017-06-21 21:42 ` Andreas Dilger
2017-07-04 18:39 ` Theodore Ts'o
2017-07-05 17:30 ` Tahsin Erdogan
2017-07-06 4:19 ` Theodore Ts'o
2017-05-31 8:15 ` [PATCH 28/28] quota: add extra inode count to dquot transfer functions Tahsin Erdogan
2017-06-15 7:57 ` Jan Kara
2017-06-15 7:57 ` [Ocfs2-devel] " Jan Kara
2017-06-17 1:50 ` Tahsin Erdogan
2017-06-19 9:03 ` Jan Kara
2017-06-19 9:03 ` [Ocfs2-devel] " Jan Kara
2017-06-19 11:46 ` Tahsin Erdogan
2017-06-19 12:36 ` Jan Kara
2017-06-19 12:36 ` [Ocfs2-devel] " Jan Kara
2017-06-20 9:12 ` [PATCH v2 28/31] quota: add get_inode_usage callback to transfer multi-inode charges Tahsin Erdogan
2017-06-20 12:01 ` Tahsin Erdogan
2017-06-20 15:28 ` Jan Kara
2017-06-20 18:08 ` [PATCH v3 " Tahsin Erdogan
2017-06-21 4:48 ` Theodore Ts'o
2017-06-21 11:22 ` Tahsin Erdogan
2017-06-20 9:53 ` [PATCH 28/28] quota: add extra inode count to dquot transfer functions Tahsin Erdogan
2017-05-31 16:42 ` [PATCH 01/28] ext4: xattr-in-inode support Darrick J. Wong
2017-05-31 16:42 ` [Ocfs2-devel] " Darrick J. Wong
2017-05-31 16:42 ` Darrick J. Wong
2017-05-31 19:59 ` Tahsin Erdogan
2017-06-01 15:50 ` [PATCH v2 " Tahsin Erdogan
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=3DC421C9-456D-4A96-AE38-2F8516F62B91@dilger.ca \
--to=adilger@dilger.ca \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tahsin@google.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.