* [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx @ 2021-09-13 7:24 Yue Hu 2021-09-13 8:48 ` Gao Xiang 0 siblings, 1 reply; 7+ messages in thread From: Yue Hu @ 2021-09-13 7:24 UTC (permalink / raw) To: xiang, chao; +Cc: huyue2, linux-erofs, linux-kernel, zhangwen, zbestahu 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. 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 7:24 [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx Yue Hu @ 2021-09-13 8:48 ` Gao Xiang 2021-09-13 9:00 ` Yue Hu 0 siblings, 1 reply; 7+ messages in thread From: Gao Xiang @ 2021-09-13 8:48 UTC (permalink / raw) To: Yue Hu; +Cc: linux-kernel, zbestahu, huyue2, linux-erofs, zhangwen 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.) 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 8:48 ` Gao Xiang @ 2021-09-13 9:00 ` Yue Hu 2021-09-13 9:11 ` Gao Xiang 0 siblings, 1 reply; 7+ messages in thread From: Yue Hu @ 2021-09-13 9:00 UTC (permalink / raw) To: Gao Xiang; +Cc: linux-kernel, zbestahu, huyue2, linux-erofs, zhangwen 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. 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 9:00 ` Yue Hu @ 2021-09-13 9:11 ` Gao Xiang 2021-09-13 10:58 ` Yue Hu 0 siblings, 1 reply; 7+ messages in thread From: Gao Xiang @ 2021-09-13 9:11 UTC (permalink / raw) To: Yue Hu; +Cc: linux-kernel, zbestahu, huyue2, linux-erofs, zhangwen 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? 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; } 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 9:11 ` Gao Xiang @ 2021-09-13 10:58 ` Yue Hu 2021-09-13 11:19 ` Gao Xiang 0 siblings, 1 reply; 7+ messages in thread From: Yue Hu @ 2021-09-13 10:58 UTC (permalink / raw) To: Gao Xiang; +Cc: linux-kernel, huyue2, linux-erofs, zhangwen 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 10:58 ` Yue Hu @ 2021-09-13 11:19 ` Gao Xiang 2021-09-13 12:22 ` Yue Hu 0 siblings, 1 reply; 7+ messages in thread From: Gao Xiang @ 2021-09-13 11:19 UTC (permalink / raw) To: Yue Hu; +Cc: linux-kernel, huyue2, linux-erofs, zhangwen Hi Yue, On Mon, Sep 13, 2021 at 06:58:36PM +0800, Yue Hu wrote: > 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. Okay, make sense. If the number of compacted_2b misleads, how about just if ((vi->z_advise & Z_EROFS_ADVISE_COMPACTED_2B) && compacted_4b_initial <= totalidx) { compacted_2b = ...; } else { compacted_2b = 0; } , and refine the commit message to point out the following facts for other folks: - compacted_4b_initial is used contain the very first lclusters in order to fulfill the alignment of the first compacted_2b pack; - compacted_4b_end is used for the last lclusters which aren't fitted in the previous compacted_2b packs; - if compacted_4b_initial > totalidx, the whole indexes will be compacted 4B and handled with compacted_4b_initial. Thanks, Gao Xiang > > Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx 2021-09-13 11:19 ` Gao Xiang @ 2021-09-13 12:22 ` Yue Hu 0 siblings, 0 replies; 7+ messages in thread From: Yue Hu @ 2021-09-13 12:22 UTC (permalink / raw) To: Gao Xiang; +Cc: linux-kernel, huyue2, linux-erofs, zhangwen On Mon, 13 Sep 2021 19:19:16 +0800 Gao Xiang <hsiangkao@linux.alibaba.com> wrote: > Hi Yue, > > On Mon, Sep 13, 2021 at 06:58:36PM +0800, Yue Hu wrote: > > 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. > > Okay, make sense. If the number of compacted_2b misleads, how about > just > > if ((vi->z_advise & Z_EROFS_ADVISE_COMPACTED_2B) && > compacted_4b_initial <= totalidx) { > compacted_2b = ...; > } else { > compacted_2b = 0; > } Looks good. So, the patch title may change a little. > > , and refine the commit message to point out the following facts for > other folks: > > - compacted_4b_initial is used contain the very first lclusters in order > to fulfill the alignment of the first compacted_2b pack; > > - compacted_4b_end is used for the last lclusters which aren't fitted in > the previous compacted_2b packs; > > - if compacted_4b_initial > totalidx, the whole indexes will be compacted > 4B and handled with compacted_4b_initial. Ok, will add these facts above in v2. Thanks. > > Thanks, > Gao Xiang > > > > > Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-13 12:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-13 7:24 [PATCH] erofs: fix compacted_{4b_initial, 2b} when compacted_4b_initial > totalidx Yue Hu 2021-09-13 8:48 ` Gao Xiang 2021-09-13 9:00 ` Yue Hu 2021-09-13 9:11 ` Gao Xiang 2021-09-13 10:58 ` Yue Hu 2021-09-13 11:19 ` Gao Xiang 2021-09-13 12:22 ` Yue Hu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).