
<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><br><div dir="ltr"><blockquote type="cite">在 2021年1月19日，23:43，Gao Xiang <hsiangkao@redhat.com> 写道：<br><br></blockquote></div><blockquote type="cite"><div dir="ltr"><span>Hi Weiwen,</span><br><span></span><br><span>On Tue, Jan 19, 2021 at 02:02:56PM +0800, 胡玮文 wrote:</span><br><blockquote type="cite"><span>Hi Xiang,</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>After further investgate, this bug will not reveal in any released version of</span><br></blockquote><blockquote type="cite"><span>mkfs.erofs. Previous patch v5 [1] will map all allocated bb when erofs_mapbh()</span><br></blockquote><blockquote type="cite"><span>is called on an already mapped bb, which triggers this bug. before that patch,</span><br></blockquote><blockquote type="cite"><span>under the same condition, __erofs_battach() will only be called on bb which is</span><br></blockquote><blockquote type="cite"><span>not mapped, thus no need to update `tail_blkaddr'.</span><br></blockquote><span></span><br><span>Good to know this, thanks! I haven't looked into that (I will test it this</span><br><span>weekend.) IMO, although this is not a regression, yet it seems it's potential</span><br><span>harmful if we didn't notice this... So I think a proper testcase is still</span><br><span>useful to look after this... If you have extra time, could you help on it?</span><br></div></blockquote><div><br></div><div>Hi Xiang,</div><div><br></div><div>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 <a href="https://github.com/huww98/erofs-utils/tree/experimental-tests">https://github.com/huww98/erofs-utils/tree/experimental-tests</a>. I will send patches once everything is sorted out.</div><br><blockquote type="cite"><div dir="ltr"><span>Also, without the detail of this, I think the fix might be folded into</span><br><span>the original patchset (could you resend it?). In addition, I think after</span><br></div></blockquote><div><br></div><div>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.</div><br><blockquote type="cite"><div dir="ltr"><span>last_mapped_block is introduced, we might not need tail_blkaddr anymore,</span><br><span>not sure. But I'm very cautious about this in case of introducing any</span><br><span>new regression...</span><br></div></blockquote><div><br></div>I think we still need it, because already mapped bb may be dropped, last_map_block does not always reflect tail_blkaddr.<div><br></div><div>Hu Weiwen<br><br><blockquote type="cite"><div dir="ltr"><span>Thanks,</span><br><span>Gao Xiang</span><br><span></span><br><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>[1]: https://lore.kernel.org/linux-erofs/20210118123431.22533-1-sehuww@mail.scut.edu.cn/</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote><blockquote type="cite"><span>Hu Weiwen</span><br></blockquote><blockquote type="cite"><span></span><br></blockquote></div></blockquote></div></body></html>