* [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file @ 2021-07-22 3:25 Fengnan Chang 2021-07-22 13:47 ` Chao Yu 0 siblings, 1 reply; 8+ messages in thread From: Fengnan Chang @ 2021-07-22 3:25 UTC (permalink / raw) To: jaegeuk, chao, linux-f2fs-devel; +Cc: Fengnan Chang Since cluster is basic unit of compression, one cluster is compressed or not, so we can calculate valid blocks only for first page in cluster, the other pages just skip. Signed-off-by: Fengnan Chang <changfengnan@vivo.com> --- fs/f2fs/data.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index d2cf48c5a2e4..a0099d8329f0 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode *inode, if (ret) goto set_error_page; } - ret = f2fs_is_compressed_cluster(inode, page->index); - if (ret < 0) - goto set_error_page; - else if (!ret) - goto read_single_page; - + if (cc.cluster_idx == NULL_CLUSTER) { + ret = f2fs_is_compressed_cluster(inode, page->index); + if (ret < 0) + goto set_error_page; + else if (!ret) + goto read_single_page; + } ret = f2fs_init_compress_ctx(&cc); if (ret) goto set_error_page; -- 2.29.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-07-22 3:25 [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file Fengnan Chang @ 2021-07-22 13:47 ` Chao Yu 2021-07-23 3:18 ` Fengnan Chang 0 siblings, 1 reply; 8+ messages in thread From: Chao Yu @ 2021-07-22 13:47 UTC (permalink / raw) To: Fengnan Chang, jaegeuk, linux-f2fs-devel On 2021/7/22 11:25, Fengnan Chang wrote: > Since cluster is basic unit of compression, one cluster is compressed or > not, so we can calculate valid blocks only for first page in cluster, the > other pages just skip. > > Signed-off-by: Fengnan Chang <changfengnan@vivo.com> > --- > fs/f2fs/data.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index d2cf48c5a2e4..a0099d8329f0 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode *inode, > if (ret) > goto set_error_page; > } > - ret = f2fs_is_compressed_cluster(inode, page->index); > - if (ret < 0) > - goto set_error_page; > - else if (!ret) > - goto read_single_page; How about truncation races with read? Thanks, > - > + if (cc.cluster_idx == NULL_CLUSTER) { > + ret = f2fs_is_compressed_cluster(inode, page->index); > + if (ret < 0) > + goto set_error_page; > + else if (!ret) > + goto read_single_page; > + } > ret = f2fs_init_compress_ctx(&cc); > if (ret) > goto set_error_page; > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-07-22 13:47 ` Chao Yu @ 2021-07-23 3:18 ` Fengnan Chang 2021-08-06 0:57 ` Chao Yu 0 siblings, 1 reply; 8+ messages in thread From: Fengnan Chang @ 2021-07-23 3:18 UTC (permalink / raw) To: Chao Yu, jaegeuk, linux-f2fs-devel f2fs_read_multi_pages will handle，all truncate page will be zero out, Whether partial or all page in cluster. On 2021/7/22 21:47, Chao Yu wrote: > On 2021/7/22 11:25, Fengnan Chang wrote: >> Since cluster is basic unit of compression, one cluster is compressed or >> not, so we can calculate valid blocks only for first page in cluster, the >> other pages just skip. >> >> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >> --- >> fs/f2fs/data.c | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index d2cf48c5a2e4..a0099d8329f0 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >> *inode, >> if (ret) >> goto set_error_page; >> } >> - ret = f2fs_is_compressed_cluster(inode, page->index); >> - if (ret < 0) >> - goto set_error_page; >> - else if (!ret) >> - goto read_single_page; > > How about truncation races with read? > > Thanks, > >> - >> + if (cc.cluster_idx == NULL_CLUSTER) { >> + ret = f2fs_is_compressed_cluster(inode, page->index); >> + if (ret < 0) >> + goto set_error_page; >> + else if (!ret) >> + goto read_single_page; >> + } >> ret = f2fs_init_compress_ctx(&cc); >> if (ret) >> goto set_error_page; >> > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-07-23 3:18 ` Fengnan Chang @ 2021-08-06 0:57 ` Chao Yu 2021-08-06 8:32 ` Fengnan Chang 2021-08-09 3:46 ` Fengnan Chang 0 siblings, 2 replies; 8+ messages in thread From: Chao Yu @ 2021-08-06 0:57 UTC (permalink / raw) To: Fengnan Chang, jaegeuk, linux-f2fs-devel On 2021/7/23 11:18, Fengnan Chang wrote: > f2fs_read_multi_pages will handle，all truncate page will be zero out, > Whether partial or all page in cluster. > > > On 2021/7/22 21:47, Chao Yu wrote: >> On 2021/7/22 11:25, Fengnan Chang wrote: >>> Since cluster is basic unit of compression, one cluster is compressed or >>> not, so we can calculate valid blocks only for first page in cluster, the >>> other pages just skip. >>> >>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >>> --- >>> fs/f2fs/data.c | 13 +++++++------ >>> 1 file changed, 7 insertions(+), 6 deletions(-) >>> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>> index d2cf48c5a2e4..a0099d8329f0 100644 >>> --- a/fs/f2fs/data.c >>> +++ b/fs/f2fs/data.c >>> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >>> *inode, >>> if (ret) >>> goto set_error_page; >>> } >>> - ret = f2fs_is_compressed_cluster(inode, page->index); >>> - if (ret < 0) >>> - goto set_error_page; >>> - else if (!ret) >>> - goto read_single_page; >> >> How about truncation races with read? Look into this again, it looks fine, truncation tries to grab all pages lock of cluster, but readahead has already held some/all of them, so there is no such race condition. So compressed cluster case looks fine to me, but we still need to call f2fs_is_compressed_cluster() every time for non-compressed cluster, could you please check that as well? Thanks, >> >> Thanks, >> >>> - >>> + if (cc.cluster_idx == NULL_CLUSTER) { >>> + ret = f2fs_is_compressed_cluster(inode, page->index); >>> + if (ret < 0) >>> + goto set_error_page; >>> + else if (!ret) >>> + goto read_single_page; >>> + } >>> ret = f2fs_init_compress_ctx(&cc); >>> if (ret) >>> goto set_error_page; >>> >> _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-08-06 0:57 ` Chao Yu @ 2021-08-06 8:32 ` Fengnan Chang 2021-08-09 3:46 ` Fengnan Chang 1 sibling, 0 replies; 8+ messages in thread From: Fengnan Chang @ 2021-08-06 8:32 UTC (permalink / raw) To: Chao Yu, jaegeuk, linux-f2fs-devel I'll check this later. Thanks. On 2021/8/6 8:57, Chao Yu wrote: > On 2021/7/23 11:18, Fengnan Chang wrote: >> f2fs_read_multi_pages will handle，all truncate page will be zero out, >> Whether partial or all page in cluster. >> >> >> On 2021/7/22 21:47, Chao Yu wrote: >>> On 2021/7/22 11:25, Fengnan Chang wrote: >>>> Since cluster is basic unit of compression, one cluster is >>>> compressed or >>>> not, so we can calculate valid blocks only for first page in >>>> cluster, the >>>> other pages just skip. >>>> >>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >>>> --- >>>> fs/f2fs/data.c | 13 +++++++------ >>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>> index d2cf48c5a2e4..a0099d8329f0 100644 >>>> --- a/fs/f2fs/data.c >>>> +++ b/fs/f2fs/data.c >>>> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >>>> *inode, >>>> if (ret) >>>> goto set_error_page; >>>> } >>>> - ret = f2fs_is_compressed_cluster(inode, page->index); >>>> - if (ret < 0) >>>> - goto set_error_page; >>>> - else if (!ret) >>>> - goto read_single_page; >>> >>> How about truncation races with read? > > Look into this again, it looks fine, truncation tries to grab all pages > lock > of cluster, but readahead has already held some/all of them, so there is no > such race condition. > > So compressed cluster case looks fine to me, but we still need to call > f2fs_is_compressed_cluster() every time for non-compressed cluster, could > you please check that as well? > > Thanks, > >>> >>> Thanks, >>> >>>> - >>>> + if (cc.cluster_idx == NULL_CLUSTER) { >>>> + ret = f2fs_is_compressed_cluster(inode, page->index); >>>> + if (ret < 0) >>>> + goto set_error_page; >>>> + else if (!ret) >>>> + goto read_single_page; >>>> + } >>>> ret = f2fs_init_compress_ctx(&cc); >>>> if (ret) >>>> goto set_error_page; >>>> >>> > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-08-06 0:57 ` Chao Yu 2021-08-06 8:32 ` Fengnan Chang @ 2021-08-09 3:46 ` Fengnan Chang 2021-08-09 14:38 ` Chao Yu 1 sibling, 1 reply; 8+ messages in thread From: Fengnan Chang @ 2021-08-09 3:46 UTC (permalink / raw) To: Chao Yu, jaegeuk, linux-f2fs-devel Hi chao: Since cc.cluster_idx only will be set in f2fs_compress_ctx_add_page, so for non-compressed cluster, cc.cluster_idx should always be NULL. it means that the handling process of non-compressed cluster is same as older. On 2021/8/6 8:57, Chao Yu wrote: > On 2021/7/23 11:18, Fengnan Chang wrote: >> f2fs_read_multi_pages will handle，all truncate page will be zero out, >> Whether partial or all page in cluster. >> >> >> On 2021/7/22 21:47, Chao Yu wrote: >>> On 2021/7/22 11:25, Fengnan Chang wrote: >>>> Since cluster is basic unit of compression, one cluster is >>>> compressed or >>>> not, so we can calculate valid blocks only for first page in >>>> cluster, the >>>> other pages just skip. >>>> >>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >>>> --- >>>> fs/f2fs/data.c | 13 +++++++------ >>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>> index d2cf48c5a2e4..a0099d8329f0 100644 >>>> --- a/fs/f2fs/data.c >>>> +++ b/fs/f2fs/data.c >>>> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >>>> *inode, >>>> if (ret) >>>> goto set_error_page; >>>> } >>>> - ret = f2fs_is_compressed_cluster(inode, page->index); >>>> - if (ret < 0) >>>> - goto set_error_page; >>>> - else if (!ret) >>>> - goto read_single_page; >>> >>> How about truncation races with read? > > Look into this again, it looks fine, truncation tries to grab all pages > lock > of cluster, but readahead has already held some/all of them, so there is no > such race condition. > > So compressed cluster case looks fine to me, but we still need to call > f2fs_is_compressed_cluster() every time for non-compressed cluster, could > you please check that as well? > > Thanks, > >>> >>> Thanks, >>> >>>> - >>>> + if (cc.cluster_idx == NULL_CLUSTER) { >>>> + ret = f2fs_is_compressed_cluster(inode, page->index); >>>> + if (ret < 0) >>>> + goto set_error_page; >>>> + else if (!ret) >>>> + goto read_single_page; >>>> + } >>>> ret = f2fs_init_compress_ctx(&cc); >>>> if (ret) >>>> goto set_error_page; >>>> >>> > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-08-09 3:46 ` Fengnan Chang @ 2021-08-09 14:38 ` Chao Yu 2021-08-10 1:50 ` Fengnan Chang 0 siblings, 1 reply; 8+ messages in thread From: Chao Yu @ 2021-08-09 14:38 UTC (permalink / raw) To: Fengnan Chang, jaegeuk, linux-f2fs-devel On 2021/8/9 11:46, Fengnan Chang wrote: > Hi chao: > Since cc.cluster_idx only will be set in f2fs_compress_ctx_add_page, > so for non-compressed cluster, cc.cluster_idx should always be NULL. it > means that the handling process of non-compressed cluster is same as older. Yup, so what I mean is why not skipping f2fs_is_compressed_cluster() check for non-compressed cluster as we did for compressed cluster. Thanks, > > On 2021/8/6 8:57, Chao Yu wrote: >> On 2021/7/23 11:18, Fengnan Chang wrote: >>> f2fs_read_multi_pages will handle，all truncate page will be zero out, >>> Whether partial or all page in cluster. >>> >>> >>> On 2021/7/22 21:47, Chao Yu wrote: >>>> On 2021/7/22 11:25, Fengnan Chang wrote: >>>>> Since cluster is basic unit of compression, one cluster is >>>>> compressed or >>>>> not, so we can calculate valid blocks only for first page in >>>>> cluster, the >>>>> other pages just skip. >>>>> >>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >>>>> --- >>>>> fs/f2fs/data.c | 13 +++++++------ >>>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>>> index d2cf48c5a2e4..a0099d8329f0 100644 >>>>> --- a/fs/f2fs/data.c >>>>> +++ b/fs/f2fs/data.c >>>>> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >>>>> *inode, >>>>> if (ret) >>>>> goto set_error_page; >>>>> } >>>>> - ret = f2fs_is_compressed_cluster(inode, page->index); >>>>> - if (ret < 0) >>>>> - goto set_error_page; >>>>> - else if (!ret) >>>>> - goto read_single_page; >>>> >>>> How about truncation races with read? >> >> Look into this again, it looks fine, truncation tries to grab all pages >> lock >> of cluster, but readahead has already held some/all of them, so there is no >> such race condition. >> >> So compressed cluster case looks fine to me, but we still need to call >> f2fs_is_compressed_cluster() every time for non-compressed cluster, could >> you please check that as well? >> >> Thanks, >> >>>> >>>> Thanks, >>>> >>>>> - >>>>> + if (cc.cluster_idx == NULL_CLUSTER) { >>>>> + ret = f2fs_is_compressed_cluster(inode, page->index); >>>>> + if (ret < 0) >>>>> + goto set_error_page; >>>>> + else if (!ret) >>>>> + goto read_single_page; >>>>> + } >>>>> ret = f2fs_init_compress_ctx(&cc); >>>>> if (ret) >>>>> goto set_error_page; >>>>> >>>> >> _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file 2021-08-09 14:38 ` Chao Yu @ 2021-08-10 1:50 ` Fengnan Chang 0 siblings, 0 replies; 8+ messages in thread From: Fengnan Chang @ 2021-08-10 1:50 UTC (permalink / raw) To: Chao Yu, jaegeuk, linux-f2fs-devel Um, in previous thought, consider of random read for non-compressed cluster, I didn't handle non-compressed cluster, after your remind, I think we can skip f2fs_is_compressed_cluster() check for sequential read for non-compressed cluster. On 2021/8/9 22:38, Chao Yu wrote: > On 2021/8/9 11:46, Fengnan Chang wrote: >> Hi chao: >> Since cc.cluster_idx only will be set in f2fs_compress_ctx_add_page, >> so for non-compressed cluster, cc.cluster_idx should always be NULL. it >> means that the handling process of non-compressed cluster is same as >> older. > > Yup, so what I mean is why not skipping f2fs_is_compressed_cluster() check > for non-compressed cluster as we did for compressed cluster. > > Thanks, > >> >> On 2021/8/6 8:57, Chao Yu wrote: >>> On 2021/7/23 11:18, Fengnan Chang wrote: >>>> f2fs_read_multi_pages will handle，all truncate page will be zero out, >>>> Whether partial or all page in cluster. >>>> >>>> >>>> On 2021/7/22 21:47, Chao Yu wrote: >>>>> On 2021/7/22 11:25, Fengnan Chang wrote: >>>>>> Since cluster is basic unit of compression, one cluster is >>>>>> compressed or >>>>>> not, so we can calculate valid blocks only for first page in >>>>>> cluster, the >>>>>> other pages just skip. >>>>>> >>>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com> >>>>>> --- >>>>>> fs/f2fs/data.c | 13 +++++++------ >>>>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>>>> index d2cf48c5a2e4..a0099d8329f0 100644 >>>>>> --- a/fs/f2fs/data.c >>>>>> +++ b/fs/f2fs/data.c >>>>>> @@ -2304,12 +2304,13 @@ static int f2fs_mpage_readpages(struct inode >>>>>> *inode, >>>>>> if (ret) >>>>>> goto set_error_page; >>>>>> } >>>>>> - ret = f2fs_is_compressed_cluster(inode, page->index); >>>>>> - if (ret < 0) >>>>>> - goto set_error_page; >>>>>> - else if (!ret) >>>>>> - goto read_single_page; >>>>> >>>>> How about truncation races with read? >>> >>> Look into this again, it looks fine, truncation tries to grab all pages >>> lock >>> of cluster, but readahead has already held some/all of them, so there >>> is no >>> such race condition. >>> >>> So compressed cluster case looks fine to me, but we still need to call >>> f2fs_is_compressed_cluster() every time for non-compressed cluster, >>> could >>> you please check that as well? >>> >>> Thanks, >>> >>>>> >>>>> Thanks, >>>>> >>>>>> - >>>>>> + if (cc.cluster_idx == NULL_CLUSTER) { >>>>>> + ret = f2fs_is_compressed_cluster(inode, >>>>>> page->index); >>>>>> + if (ret < 0) >>>>>> + goto set_error_page; >>>>>> + else if (!ret) >>>>>> + goto read_single_page; >>>>>> + } >>>>>> ret = f2fs_init_compress_ctx(&cc); >>>>>> if (ret) >>>>>> goto set_error_page; >>>>>> >>>>> >>> > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-10 1:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-22 3:25 [f2fs-dev] [RFC PATCH] f2fs: compress: avoid duplicate counting of valid blocks when read compressed file Fengnan Chang 2021-07-22 13:47 ` Chao Yu 2021-07-23 3:18 ` Fengnan Chang 2021-08-06 0:57 ` Chao Yu 2021-08-06 8:32 ` Fengnan Chang 2021-08-09 3:46 ` Fengnan Chang 2021-08-09 14:38 ` Chao Yu 2021-08-10 1:50 ` Fengnan Chang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.