Re: [PATCH 1/2 v2] AOSP: erofs-utils: pass a parameter to write tail end in block list

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

On 2023/8/15 14:21, Yue Hu wrote:
> On Tue, 15 Aug 2023 13:36:46 +0800
> Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> 
>> On 2023/8/15 13:40, Yue Hu wrote:
>>> On Tue, 15 Aug 2023 12:59:56 +0800
>>> Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>>>    
>>>> On 2023/8/15 12:55, Yue Hu wrote:
>>>>> From: Yue Hu <huyue2@coolpad.com>
>>>>>
>>>>> We can determine whether the tail block is the first one or not during
>>>>> the writing process.  Therefore, instead of internally checking the
>>>>> block number for the tail block map, just simply pass the flag.
>>>>>
>>>>> Also, add the missing sbi argument to macro erofs_blknr.
>>>>
>>>> Could you submit a patch to fix this issue first?
>>>
>>> ok, will do that.
>>>    
>>>>   
>>>>>
>>>>> Signed-off-by: Yue Hu <huyue2@coolpad.com>
>>>>> ---
>>>>> v2: change commit message a bit
>>>>>
>>>>>     include/erofs/block_list.h | 4 ++--
>>>>>     lib/block_list.c           | 5 ++---
>>>>>     lib/inode.c                | 9 +++++++--
>>>>>     3 files changed, 11 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/include/erofs/block_list.h b/include/erofs/block_list.h
>>>>> index 78fab44..e0dced8 100644
>>>>> --- a/include/erofs/block_list.h
>>>>> +++ b/include/erofs/block_list.h
>>>>> @@ -19,7 +19,7 @@ void erofs_droid_blocklist_fclose(void);
>>>>>     void erofs_droid_blocklist_write(struct erofs_inode *inode,
>>>>>     				 erofs_blk_t blk_start, erofs_blk_t nblocks);
>>>>>     void erofs_droid_blocklist_write_tail_end(struct erofs_inode *inode,
>>>>> -					  erofs_blk_t blkaddr);
>>>>> +					  erofs_blk_t blkaddr, bool first_block);
>>>>
>>>> I still have no idea why we need this, could you describe the Android
>>>> block map details for discussion?
>>>
>>> Android block map is just adding file blocks to a range.
>>>
>>> So, the tail block should be needed in this range as well.
>>> I think one simple way is just appending the tail block address in it as below:
>>>
>>> /`file_path` `block1_address`-`blockn_address` `tail_block_address`
>>
>> why `tail_block_address` needs a seperate field?
> 
> Well, `erofs_write_tail_end()` is a separate logic and i think appending this block is
> simple enough since i don't need to consider whether the block address is contiguous
> with previous one.
> 
> And i think Android block map can handle this since i have saw below in ext4:
> 
> /system/.../libclang_rt.ubsan_standalone-arm-android.so 51276-51309 0 51310-51403

What's the meaning of 0 here?

Thanks,
Gao Xiang