>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
Hi, yong
Thanks for sharing what you proposed to do. By the way, I noticed that
when setattr, read_all_xattr will set xattr header's magic and refcount,
but it seems we don't check these values in header(kernel/fsck). Should we
add check logic for it?
>> + 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
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
