Date:   Tue, 17 Mar 2020 12:47:12 +0100
From:   Jan Kara <jack@...e.cz>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Linux FS Devel <linux-fsdevel@...r.kernel.org>,
        linux-ext4@...r.kernel.org, Jan Kara <jack@...e.com>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] ext2: fix debug reference to ext2_xattr_cache

On Fri 13-03-20 20:42:05, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@...radead.org>
> 
> Fix a debug-only build error in ext2/xattr.c:
> 
> When building without extra debugging, (and with another patch that uses
> no_printk() instead of <empty> for the ext2-xattr debug-print macros,
> this build error happens:
> 
> ../fs/ext2/xattr.c: In function ‘ext2_xattr_cache_insert’:
> ../fs/ext2/xattr.c:869:18: error: ‘ext2_xattr_cache’ undeclared (first use in this function); did you mean ‘ext2_xattr_list’?
>      atomic_read(&ext2_xattr_cache->c_entry_count));
> 
> Fix by moving struct mb_cache from fs/mbcache.c to <linux/mbcache.h>,
> and then using the correct struct name in the debug-print macro.
> 
> This wasn't converted when ext2 xattr cache was changed to use mbcache.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> Cc: Jan Kara <jack@...e.com>
> Cc: Alexander Viro <viro@...iv.linux.org.uk>
> Cc: linux-ext4@...r.kernel.org
> Cc: linux-fsdevel@...r.kernel.org

Thanks for the patch! I don't think exporting 'struct mb_cache' just for
this is reasonable. I've committed a patch which just removes the entry
count from the debug message (attached).

								Honza

> ---
> This is ancient, from the beginning of git history.
> 
> Or just kill of that print of c_entry_count...
> 
>  fs/ext2/xattr.c         |    2 +-
>  fs/mbcache.c            |   34 ----------------------------------
>  include/linux/mbcache.h |   35 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 35 insertions(+), 36 deletions(-)
> 
> --- linux-next-20200313.orig/fs/ext2/xattr.c
> +++ linux-next-20200313/fs/ext2/xattr.c
> @@ -873,7 +873,7 @@ ext2_xattr_cache_insert(struct mb_cache
>  	if (error) {
>  		if (error == -EBUSY) {
>  			ea_bdebug(bh, "already in cache (%d cache entries)",
> -				atomic_read(&ext2_xattr_cache->c_entry_count));
> +				atomic_read(&cache->c_entry_count));
>  			error = 0;
>  		}
>  	} else
> --- linux-next-20200313.orig/fs/mbcache.c
> +++ linux-next-20200313/fs/mbcache.c
> @@ -8,40 +8,6 @@
>  #include <linux/workqueue.h>
>  #include <linux/mbcache.h>
>  
> -/*
> - * 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()).
> - *
> - * Ext2 and ext4 use this cache for deduplication of extended attribute blocks.
> - * Ext4 also uses it for deduplication of xattr values stored in inodes.
> - * They use hash of data as a key and provide a value that may represent a
> - * block or inode number. That's why keys need not be unique (hash of different
> - * data may be the same). However user provided value always uniquely
> - * identifies a cache entry.
> - *
> - * We provide functions for creation and removal of entries, search by key,
> - * and a special "delete entry with given key-value pair" operation. Fixed
> - * size hash table is used for fast key lookups.
> - */
> -
> -struct mb_cache {
> -	/* Hash table of entries */
> -	struct hlist_bl_head	*c_hash;
> -	/* log2 of hash table size */
> -	int			c_bucket_bits;
> -	/* Maximum entries in cache to avoid degrading hash too much */
> -	unsigned long		c_max_entries;
> -	/* Protects c_list, c_entry_count */
> -	spinlock_t		c_list_lock;
> -	struct list_head	c_list;
> -	/* Number of entries in cache */
> -	unsigned long		c_entry_count;
> -	struct shrinker		c_shrink;
> -	/* Work for shrinking when the cache has too many entries */
> -	struct work_struct	c_shrink_work;
> -};
> -
>  static struct kmem_cache *mb_entry_cache;
>  
>  static unsigned long mb_cache_shrink(struct mb_cache *cache,
> --- linux-next-20200313.orig/include/linux/mbcache.h
> +++ linux-next-20200313/include/linux/mbcache.h
> @@ -8,7 +8,40 @@
>  #include <linux/atomic.h>
>  #include <linux/fs.h>
>  
> -struct mb_cache;
> +/*
> + * 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()).
> + *
> + * Ext2 and ext4 use this cache for deduplication of extended attribute blocks.
> + * Ext4 also uses it for deduplication of xattr values stored in inodes.
> + * They use hash of data as a key and provide a value that may represent a
> + * block or inode number. That's why keys need not be unique (hash of different
> + * data may be the same). However user provided value always uniquely
> + * identifies a cache entry.
> + *
> + * We provide functions for creation and removal of entries, search by key,
> + * and a special "delete entry with given key-value pair" operation. Fixed
> + * size hash table is used for fast key lookups.
> + */
> +
> +struct mb_cache {
> +	/* Hash table of entries */
> +	struct hlist_bl_head	*c_hash;
> +	/* log2 of hash table size */
> +	int			c_bucket_bits;
> +	/* Maximum entries in cache to avoid degrading hash too much */
> +	unsigned long		c_max_entries;
> +	/* Protects c_list, c_entry_count */
> +	spinlock_t		c_list_lock;
> +	struct list_head	c_list;
> +	/* Number of entries in cache */
> +	unsigned long		c_entry_count;
> +	struct shrinker		c_shrink;
> +	/* Work for shrinking when the cache has too many entries */
> +	struct work_struct	c_shrink_work;
> +};
> +
>  
>  struct mb_cache_entry {
>  	/* List of entries in cache - protected by cache->c_list_lock */
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

View attachment "0001-ext2-fix-debug-reference-to-ext2_xattr_cache.patch" of type "text/x-patch" (1596 bytes)

Powered by blists - more mailing lists