Hi Xiang, On Wed, Jan 20, 2021 at 01:12:16PM +0800, Gao Xiang wrote: > Hi Weiwen, > > On Wed, Jan 20, 2021 at 12:57:39PM +0800, 胡玮文 wrote: > > > > > 在 2021年1月19日，23:43，Gao Xiang <[email protected]> 写道： > > > > > > Hi Weiwen, > > > > > >> On Tue, Jan 19, 2021 at 02:02:56PM +0800, 胡玮文 wrote: > > >> Hi Xiang, > > >> > > >> After further investgate, this bug will not reveal in any released > > >> version of > > >> mkfs.erofs. Previous patch v5 [1] will map all allocated bb when > > >> erofs_mapbh() > > >> is called on an already mapped bb, which triggers this bug. before that > > >> patch, > > >> under the same condition, __erofs_battach() will only be called on bb > > >> which is > > >> not mapped, thus no need to update `tail_blkaddr'. > > > > > > Good to know this, thanks! I haven't looked into that (I will test it this > > > weekend.) IMO, although this is not a regression, yet it seems it's > > > potential > > > harmful if we didn't notice this... So I think a proper testcase is still > > > useful to look after this... If you have extra time, could you help on it? > > > > Hi Xiang, > > > > I’m working on this. I have written a test case for this. And I’m also > > working on setting up GitHub actions to run tests automatically. So far, > > I’ve got uncompressed tests works, but when lz4 is enable, all test (except > > 001) fail. I have not found out why. You may see my progress at > > https://github.com/huww98/erofs-utils/tree/experimental-tests. I will send > > patches once everything is sorted out. > > It would be better to know which kernel version github action is used (at > least > it seems no good if version is < 5.4)? also could you confirm the lz4 version > as well (lz4-1.9.3)? if erofsfuse is used, specify "FSTYP=erofsfuse make > check" > to test it.
I've verified kernel version is 5.4.0-1032-azure for ubuntu-20.04, and erofs mount succeeded. for lz4, I'm installing v1.9.3 manually from source. I haven't tried fuse, will give it a try later. > The temporary results are in "tests/results/", could you also check and debug > it? (please kindly confirm the testcases work well on your local computer, > since such testcase is still WIP, I'm not sure if it has some running issues > as well) I've downloaded "tests/results/" and it's test 007 (check for bad lz4 versions) that fails with output "test LZ4_compress_HC_destSize(1048576) error (4098 < 4116)". And it's the same error on my PC. Investigating. BTW, why not use a more meaningful name for each test rather than a sequence number? Hu Weiwen > > > > > Also, without the detail of this, I think the fix might be folded into > > > the original patchset (could you resend it?). In addition, I think after > > > > You mean add a new commit [PATCH v6 3/3], or merge it into [PATCH v7 2/2]? > > I send it as a separate patch set because it may be merged independent of > > the cache.c optimization. > > Resend v7 and fold it into [v7 2/2] would be better... > > > > > > last_mapped_block is introduced, we might not need tail_blkaddr anymore, > > > not sure. But I'm very cautious about this in case of introducing any > > > new regression... > > > > I think we still need it, because already mapped bb may be dropped, > > last_map_block does not always reflect tail_blkaddr. > > Okay, that makes sense... > > Thanks, > Gao Xiang > > > > > Hu Weiwen > > > > > Thanks, > > > Gao Xiang > > > > > >> > > >> [1]: > > >> https://lore.kernel.org/linux-erofs/[email protected]/ > > >> > > >> Hu Weiwen > > >>
