Messages in this thread | | | Date | Mon, 13 Sep 2021 18:58:36 +0800 | From | Yue Hu <> | Subject | Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx |
| |
Hi Xiang,
On Mon, 13 Sep 2021 17:11:24 +0800 Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> On Mon, Sep 13, 2021 at 05:00:16PM +0800, Yue Hu wrote: > > On Mon, 13 Sep 2021 16:48:45 +0800 > > Gao Xiang <hsiangkao@linux.alibaba.com> wrote: > > > > > Hi Yue, > > > > > > On Mon, Sep 13, 2021 at 03:24:05PM +0800, Yue Hu wrote: > > > > From: Yue Hu <huyue2@yulong.com> > > > > > > > > mkfs.erofs will treat compacted_4b_initial & compacted_2b as 0 if > > > > compacted_4b_initial > totalidx, kernel should be aligned with it > > > > accordingly. > > > > > > There is no difference between compacted_4b_initial or compacted_4b_end > > > for compacted 4B. Since in this way totalidx for compact 2B won't larger > > > than 16 (number of lclusters in a compacted 2B pack.) > > > > However, we can see compacted_2b is a big number for this case. It should > > be pointless. > > Does it has some real impact?
No real impact to correct result.
> > compacted_4b_initial is only used for the alignment use for the > first compacted_2b so that each compacted_2b pack won't cross > the block (page) boundary. And compacted_4b_end is for the last > lclusters aren't fitted in any compacted_2b pack. > > If compacted_4b_initial > totalidx, I think the whole indexes > would be compacted 4B and handled in > > if (lcn < compacted_4b_initial) { > amortizedshift = 2; > goto out; > }
Yes, it is.
My point is why we need compacted_2b here for this case. If it's not helpful/used for next code logic, we should remove/avoid it. I think that may cause some misunderstanding and consume unneeded CPU resources.
Thanks.
> > Thanks, > Gao Xiang > > > > > Thanks. > > > > > > > > So it can be handled in either compacted_4b_initial or compacted_4b_end > > > cases, because there are all compacted 4B. > > > > > > Thanks, > > > Gao Xiang > > > > > > > > > > > Signed-off-by: Yue Hu <huyue2@yulong.com> > > > > --- > > > > fs/erofs/zmap.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c > > > > index 9fb98d8..4f941b6 100644 > > > > --- a/fs/erofs/zmap.c > > > > +++ b/fs/erofs/zmap.c > > > > @@ -369,7 +369,10 @@ static int compacted_load_cluster_from_disk(struct z_erofs_maprecorder *m, > > > > if (compacted_4b_initial == 32 / 4) > > > > compacted_4b_initial = 0; > > > > > > > > - if (vi->z_advise & Z_EROFS_ADVISE_COMPACTED_2B) > > > > + if (compacted_4b_initial > totalidx) { > > > > + compacted_4b_initial = 0; > > > > + compacted_2b = 0; > > > > + } else if (vi->z_advise & Z_EROFS_ADVISE_COMPACTED_2B) > > > > compacted_2b = rounddown(totalidx - compacted_4b_initial, 16); > > > > else > > > > compacted_2b = 0; > > > > -- > > > > 1.9.1
| |