On Thu, Jan 21, 2021 at 02:07:38PM +0800, 胡玮文 wrote: > 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.
OK, I realized I just need to run ldconfig after installing new version of liblz4.so > 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 > > > >>
