On Tue, 15 Aug 2023 14:44:23 +0800 Gao Xiang <[email protected]> wrote:
> On 2023/8/15 14:45, Yue Hu wrote: > > On Tue, 15 Aug 2023 14:18:08 +0800 > > Gao Xiang <[email protected]> wrote: > > > >> On 2023/8/15 14:21, Yue Hu wrote: > >>> On Tue, 15 Aug 2023 13:36:46 +0800 > >>> Gao Xiang <[email protected]> wrote: > >>> > >>>> On 2023/8/15 13:40, Yue Hu wrote: > >>>>> On Tue, 15 Aug 2023 12:59:56 +0800 > >>>>> Gao Xiang <[email protected]> wrote: > >>>>> > >>>>>> On 2023/8/15 12:55, Yue Hu wrote: > >>>>>>> From: Yue Hu <[email protected]> > >>>>>>> > >>>>>>> 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 <[email protected]> > >>>>>>> --- > >>>>>>> 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? > > > > I have not check that yet, maybe i need to find time... > > > > And check this: "/.../libxxx.so 87378-88630 0 0 0 88631 0 0 88632-88637 0 0 > > 0 88638-88663" > > > > so, i guess it looks like null block. > > So here erofs_droid_blocklist_write_tail_end is not actually needed in > principle? > 0 is just used for sparse block? We need to confirm this. > > Thanks, > Gao Xiang
