On 2023/7/5 13:39, Jingbo Xu wrote:


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 <[email protected]>
---
   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?

shared xattrs will be used for later tree walking. non shared xattrs
are not.



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

That is true, but in principle these are seperate two stuffs:

 - erofs_build_shared_xattrs_from_path  generates shared xattrs
                                        from a tree.
 - erofs_mkfs_build_tree_from_path generate trees with the
                                   previous shared xattrs.

These two paths can even be different if we'd like to build
a wide shared xattr array.

For example, you could build shared xattrs from "/usr" but only
pack "/usr/lib" then.

Thanks,
Gao Xiang



Reply via email to