linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: 胡玮文 <sehuww@mail.scut.edu.cn>
To: Gao Xiang <hsiangkao@redhat.com>
Cc: linux-erofs@lists.ozlabs.org
Subject: Re: [PATCH] erofs-utils: fix battach on full buffer block
Date: Thu, 21 Jan 2021 14:07:38 +0800	[thread overview]
Message-ID: <20210121060738.GA6680@DESKTOP-N4CECTO.huww98.cn> (raw)
In-Reply-To: <20210120051216.GA2688693@xiangao.remote.csb>

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 <hsiangkao@redhat.com> 写道:
> > > 
> > > 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/20210118123431.22533-1-sehuww@mail.scut.edu.cn/
> > >> 
> > >> Hu Weiwen
> > >> 


  reply	other threads:[~2021-01-21  6:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 12:39 Hu Weiwen
2021-01-18 13:59 ` Gao Xiang
2021-01-18 15:52   ` 胡玮文
2021-01-18 23:28     ` Gao Xiang
2021-01-19  6:02   ` 胡玮文
2021-01-19 15:43     ` Gao Xiang
2021-01-20  4:57       ` 胡玮文
2021-01-20  5:12         ` Gao Xiang
2021-01-21  6:07           ` 胡玮文 [this message]
2021-01-21  7:22             ` 胡玮文
2021-01-21  9:22             ` Gao Xiang
2021-01-21 16:26           ` [PATCH v2] " Hu Weiwen
2021-01-22  2:31             ` Gao Xiang
2021-01-22 12:43             ` Gao Xiang
2021-02-14 15:22             ` Gao Xiang via Linux-erofs
2021-02-14 16:00             ` [PATCH v3] erofs-utils: fix battach on full buffer blocks Gao Xiang via Linux-erofs
2021-02-22 15:34               ` Li GuiFu via Linux-erofs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210121060738.GA6680@DESKTOP-N4CECTO.huww98.cn \
    --to=sehuww@mail.scut.edu.cn \
    --cc=hsiangkao@redhat.com \
    --cc=linux-erofs@lists.ozlabs.org \
    --subject='Re: [PATCH] erofs-utils: fix battach on full buffer block' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox