Re: [f2fs-dev] f2fs-tools: Check and fix inline xattr inplace

[Sheng Yong via Linux-f2fs-devel]

On 2024/12/4 20:23, wangzijie wrote:

When we check inode which just has inline xattr data, we copy
inline xattr data from inode, check it(maybe fix it) and copy
it again to inode. We can check and fix xattr inplace for this
kind of inode to reduce memcpy times.

Signed-off-by: wangzijie <wangzij...@honor.com>
---
  fsck/fsck.c | 18 ++++++++++++++----
  1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index aa3fb97..fd8b082 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -840,11 +840,18 @@ int chk_extended_attributes(struct f2fs_sb_info *sbi, u32 
nid,
        struct f2fs_xattr_entry *ent;
        __u32 xattr_size = XATTR_SIZE(&inode->i);
        bool need_fix = false;
+       bool has_xattr_node = false;
+       nid_t xnid = le32_to_cpu(inode->i.i_xattr_nid);

if (xattr_size == 0)

                return 0;

- xattr = read_all_xattrs(sbi, inode, false);

+       if (xattr_size <= inline_xattr_size(&inode->i) && !xnid)

Hi, zijie,

I propose to change the behavors of read_all_xattrs and write_all_xattrs, and 
to add a
new free_xattrs.

* read_all_xattrs checks whether xnid exist. If it's not, return inline_xattr 
directly
  without alloc and memcpy.
* write_all_xattrs checks whether inline_xattr and new txattr_addr have the 
same address
  to avoid copying back.
* free_xattrs checks whether inline_xattr and new txattr_addr have the same 
address to
  free xattr buffer properly.

In this way, all instances where {read|write}_all_xattrs are called can avoid 
unnecessary
memory alloc and copy. free_xattrs(xattrs) should be used instead of 
free(xattrs).

thanks,
shengyong

+               xattr = inline_xattr_addr(&inode->i);
+       else {
+               xattr = read_all_xattrs(sbi, inode, false);
+               has_xattr_node = true;
+       }
        ASSERT(xattr);

last_base_addr = (void *)xattr + xattr_size;

@@ -867,12 +874,15 @@ int chk_extended_attributes(struct f2fs_sb_info *sbi, u32 
nid,
        }
        if (need_fix && c.fix_on) {
                memset(ent, 0, (u8 *)last_base_addr - (u8 *)ent);
-               write_all_xattrs(sbi, inode, xattr_size, xattr);
+               if (has_xattr_node) {
+                       write_all_xattrs(sbi, inode, xattr_size, xattr);
+                       free(xattr);
+               }
                FIX_MSG("[0x%x] nullify wrong xattr entries", nid);
-               free(xattr);
                return 1;
        }
-       free(xattr);
+       if (has_xattr_node)
+               free(xattr);
        return 0;
  }

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel