Re: [PATCH v6 3/3] erofs-utils: mkfs: enable xattr name filter

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2023/8/29 20:41, Jingbo Xu wrote:
> Introduce "-Exattr-name-filter" option to enable the xattr name bloom
> filter feature.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> ---
>   include/erofs/config.h   |  1 +
>   include/erofs/internal.h |  1 +
>   lib/xattr.c              | 63 ++++++++++++++++++++++++++++++++++++++++
>   mkfs/main.c              |  7 +++++
>   4 files changed, 72 insertions(+)
> 
> diff --git a/include/erofs/config.h b/include/erofs/config.h
> index 8f52d2c..c51f0cd 100644
> --- a/include/erofs/config.h
> +++ b/include/erofs/config.h
> @@ -53,6 +53,7 @@ struct erofs_configure {
>   	bool c_ignore_mtime;
>   	bool c_showprogress;
>   	bool c_extra_ea_name_prefixes;
> +	bool c_xattr_name_filter;
>   
>   #ifdef HAVE_LIBSELINUX
>   	struct selabel_handle *sehnd;
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 3e73eef..382024a 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -139,6 +139,7 @@ EROFS_FEATURE_FUNCS(fragments, incompat, INCOMPAT_FRAGMENTS)
>   EROFS_FEATURE_FUNCS(dedupe, incompat, INCOMPAT_DEDUPE)
>   EROFS_FEATURE_FUNCS(xattr_prefixes, incompat, INCOMPAT_XATTR_PREFIXES)
>   EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
> +EROFS_FEATURE_FUNCS(xattr_filter, compat, COMPAT_XATTR_FILTER)
>   
>   #define EROFS_I_EA_INITED	(1 << 0)
>   #define EROFS_I_Z_INITED	(1 << 1)
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 46a301a..325241d 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -18,6 +18,7 @@
>   #include "erofs/cache.h"
>   #include "erofs/io.h"
>   #include "erofs/fragments.h"
> +#include "erofs/xxhash.h"
>   #include "liberofs_private.h"
>   
>   #define EA_HASHTABLE_BITS 16
> @@ -783,6 +784,63 @@ out:
>   	return ret;
>   }
>   
> +
> +static int erofs_xattr_filter_hashbit(struct xattr_item *item)
> +{
> +	u8 prefix = item->prefix;
> +	const char *key = item->kvbuf;
> +	unsigned int len = item->len[0];
> +	char *name = NULL;
> +	uint32_t hashbit;
> +
> +	if (prefix & EROFS_XATTR_LONG_PREFIX) {
> +		struct ea_type_node *tnode;
> +		u16 prefix_len;
> +		int ret;
> +
> +		list_for_each_entry(tnode, &ea_name_prefixes, list) {
> +			if (tnode->index == item->prefix) {
> +				ret = asprintf(&name, "%s%.*s",
> +					       tnode->type.prefix, len, key);
> +				if (ret < 0)
> +					return -ENOMEM;
> +				break;
> +			}
> +		}
> +		if (!name)
> +			return -ENOENT;
> +
> +		if (!match_base_prefix(name, &prefix, &prefix_len)) {
> +			free(name);
> +			return -ENOENT;
> +		}
> +		key = name + prefix_len;
> +		len = strlen(key);
> +	}
> +
> +	hashbit = xxh32(key, len, EROFS_XATTR_FILTER_SEED + prefix) &
> +		  (EROFS_XATTR_FILTER_BITS - 1);
> +	if (name)
> +		free(name);
> +	return hashbit;
> +}
> +
> +static u32 erofs_xattr_filter_map(struct list_head *ixattrs)
> +{
> +	struct inode_xattr_node *node, *n;
> +	u32 name_filter;
> +	int hashbit;
> +
> +	name_filter = 0;
> +	list_for_each_entry_safe(node, n, ixattrs, list) {
> +		hashbit = erofs_xattr_filter_hashbit(node->item);
> +		if (hashbit < 0)

I'd suggest to clear feature bit instead:

erofs_sb_clear_xattr_filter(&sbi);

> +			return 0;
> +		name_filter |= (1UL << hashbit);
> +	}
> +	return EROFS_XATTR_FILTER_DEFAULT & ~name_filter;
> +}
> +
>   char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned int size)
>   {
>   	struct inode_xattr_node *node, *n;
> @@ -797,6 +855,11 @@ char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned int size)
>   	header = (struct erofs_xattr_ibody_header *)buf;
>   	header->h_shared_count = 0;
>   
> +	if (cfg.c_xattr_name_filter) {
> +		u32 name_filter = erofs_xattr_filter_map(ixattrs);

Leave a blank line here.

> +		header->h_name_filter = cpu_to_le32(name_filter);

Or
		header->h_name_filter =
			cpu_to_le32(erofs_xattr_filter_map(ixattrs));

Thanks,
Gao Xiang