[PATCH] erofs-utils: mkfs: don't clean cached xattr items prematurely

[Jingbo Xu]

On 7/5/23 10:21 AM, Gao Xiang wrote:
> 
> 
> On 2023/7/5 10:10, Jingbo Xu wrote:
>> Extended attributes read from file are cached in a hash table when
>> building shared extended attributes.  However the cached xattr items
>> for inline xattrs are cleaned up from the hash table when the processing
>> for shared extended attributes has finished, while later the hash table
>> is reconstructed from scratch when building the inode tree (see
>> erofs_mkfs_build_tree_from_path()).
>>
>> Don't clean up the xattr hash table halfway until mkfs exits.  Also move
>> the logic of cleaning long xattr name prefixes into erofs_cleanxattrs().
>>
>> Signed-off-by: Jingbo Xu <jefflexu at linux.alibaba.com>
>> ---
>>   include/erofs/xattr.h |  2 +-
>>   lib/xattr.c           | 30 ++++++++----------------------
>>   mkfs/main.c           |  2 +-
>>   3 files changed, 10 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/erofs/xattr.h b/include/erofs/xattr.h
>> index 14fc081..b202f78 100644
>> --- a/include/erofs/xattr.h
>> +++ b/include/erofs/xattr.h
>> @@ -75,9 +75,9 @@ static inline unsigned int
>> xattrblock_offset(unsigned int xattr_id)
>>   int erofs_prepare_xattr_ibody(struct erofs_inode *inode);
>>   char *erofs_export_xattr_ibody(struct list_head *ixattrs, unsigned
>> int size);
>>   int erofs_build_shared_xattrs_from_path(const char *path);
>> +void erofs_cleanxattrs(void);
>>     int erofs_xattr_insert_name_prefix(const char *prefix);
>> -void erofs_xattr_cleanup_name_prefixes(void);
>>   int erofs_xattr_write_name_prefixes(FILE *f);
>>     #ifdef __cplusplus
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 7d7dc54..8d0079f 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -547,24 +547,23 @@ fail:
>>       return ret;
>>   }
>>   -static void erofs_cleanxattrs(bool sharedxattrs)
>> +void erofs_cleanxattrs(void)
>>   {
>>       unsigned int i;
>>       struct xattr_item *item;
>>       struct hlist_node *tmp;
>> +    struct ea_type_node *tnode, *n;
>>         hash_for_each_safe(ea_hashtable, i, tmp, item, node) {
>> -        if (sharedxattrs && item->shared_xattr_id >= 0)
>> -            continue;
> 
> I'm not sure it's the expected behavior. Previously we will remove
> all non-shared xattrs which are below xattr threshold in the shared
> xattr generation process.

What's the purpose of removing these non-shared xattrs while leaving
shared xattrs there?

> 
> But now, such non-shared xattrs will be left in memory.  What
> the benefits of this?  In addition, I'm not sure if we need to add
> non-shared (inline) xattrs to hash table as well.

Later erofs_mkfs_build_tree_from_path() needs to reconstruct these
non-shared xattrs, which have been previously removed.

erofs_mkfs_build_tree_from_path
  erofs_mkfs_build_tree
    erofs_prepare_xattr_ibody
      read_xattrs_from_file


-- 
Thanks,
Jingbo