2020-07-24 10:22:44

by Chao Yu

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: compress: fix to update isize when overwriting compressed file

We missed to update isize of compressed file in write_end() with
below case:

cluster size is 16KB

- write 14KB data from offset 0
- overwrite 16KB data from offset 0

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/data.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 44645f4f914b..1d7280a70e09 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3470,6 +3470,10 @@ static int f2fs_write_end(struct file *file,
if (f2fs_compressed_file(inode) && fsdata) {
f2fs_compress_write_end(inode, fsdata, page->index, copied);
f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
+
+ if (pos + copied > i_size_read(inode) &&
+ !f2fs_verity_in_progress(inode))
+ f2fs_i_size_write(inode, pos + copied);
return copied;
}
#endif
--
2.26.2


2020-07-24 10:25:20

by Chao Yu

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: compress: delay temp page allocation

Currently, we allocate temp pages which is used to pad hole in
cluster during read IO submission, it may take long time before
releasing them in f2fs_decompress_pages(), since they are only
used as temp output buffer in decompression context, so let's
just do the allocation in that context to reduce time of memory
pool resource occupation.

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/compress.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index a20c9f3272af..337a272d7112 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -670,6 +670,7 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
const struct f2fs_compress_ops *cops =
f2fs_cops[fi->i_compress_algorithm];
int ret;
+ int i;

dec_page_count(sbi, F2FS_RD_DATA);

@@ -688,6 +689,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
goto out_free_dic;
}

+ dic->tpages = f2fs_kzalloc(sbi, sizeof(struct page *) *
+ dic->cluster_size, GFP_NOFS);
+ if (!dic->tpages)
+ goto out_free_dic;
+
+ for (i = 0; i < dic->cluster_size; i++) {
+ if (dic->rpages[i]) {
+ dic->tpages[i] = dic->rpages[i];
+ continue;
+ }
+
+ dic->tpages[i] = f2fs_compress_alloc_page();
+ if (!dic->tpages[i])
+ goto out_free_dic;
+ }
+
if (cops->init_decompress_ctx) {
ret = cops->init_decompress_ctx(dic);
if (ret)
@@ -1449,22 +1466,6 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc)
dic->cpages[i] = page;
}

- dic->tpages = f2fs_kzalloc(sbi, sizeof(struct page *) *
- dic->cluster_size, GFP_NOFS);
- if (!dic->tpages)
- goto out_free;
-
- for (i = 0; i < dic->cluster_size; i++) {
- if (cc->rpages[i]) {
- dic->tpages[i] = cc->rpages[i];
- continue;
- }
-
- dic->tpages[i] = f2fs_compress_alloc_page();
- if (!dic->tpages[i])
- goto out_free;
- }
-
return dic;

out_free:
--
2.26.2

2020-07-24 19:27:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] f2fs: compress: delay temp page allocation

Hi Chao,

I love your patch! Perhaps something to improve:

[auto build test WARNING on f2fs/dev-test]
[also build test WARNING on linux/master linus/master v5.8-rc6 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Chao-Yu/f2fs-compress-fix-to-update-isize-when-overwriting-compressed-file/20200724-182420
base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1d09ecf36175f7910ffedd6d497c07b5c74c22fb)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> fs/f2fs/compress.c:704:7: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!dic->tpages[i])
^~~~~~~~~~~~~~~
fs/f2fs/compress.c:751:19: note: uninitialized use occurs here
dic->clen, ret);
^~~
fs/f2fs/compress.c:704:3: note: remove the 'if' if its condition is always false
if (!dic->tpages[i])
^~~~~~~~~~~~~~~~~~~~
fs/f2fs/compress.c:694:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!dic->tpages)
^~~~~~~~~~~~
fs/f2fs/compress.c:751:19: note: uninitialized use occurs here
dic->clen, ret);
^~~
fs/f2fs/compress.c:694:2: note: remove the 'if' if its condition is always false
if (!dic->tpages)
^~~~~~~~~~~~~~~~~
fs/f2fs/compress.c:672:9: note: initialize the variable 'ret' to silence this warning
int ret;
^
= 0
2 warnings generated.

vim +704 fs/f2fs/compress.c

663
664 void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
665 {
666 struct decompress_io_ctx *dic =
667 (struct decompress_io_ctx *)page_private(page);
668 struct f2fs_sb_info *sbi = F2FS_I_SB(dic->inode);
669 struct f2fs_inode_info *fi= F2FS_I(dic->inode);
670 const struct f2fs_compress_ops *cops =
671 f2fs_cops[fi->i_compress_algorithm];
672 int ret;
673 int i;
674
675 dec_page_count(sbi, F2FS_RD_DATA);
676
677 if (bio->bi_status || PageError(page))
678 dic->failed = true;
679
680 if (refcount_dec_not_one(&dic->ref))
681 return;
682
683 trace_f2fs_decompress_pages_start(dic->inode, dic->cluster_idx,
684 dic->cluster_size, fi->i_compress_algorithm);
685
686 /* submit partial compressed pages */
687 if (dic->failed) {
688 ret = -EIO;
689 goto out_free_dic;
690 }
691
692 dic->tpages = f2fs_kzalloc(sbi, sizeof(struct page *) *
693 dic->cluster_size, GFP_NOFS);
694 if (!dic->tpages)
695 goto out_free_dic;
696
697 for (i = 0; i < dic->cluster_size; i++) {
698 if (dic->rpages[i]) {
699 dic->tpages[i] = dic->rpages[i];
700 continue;
701 }
702
703 dic->tpages[i] = f2fs_compress_alloc_page();
> 704 if (!dic->tpages[i])
705 goto out_free_dic;
706 }
707
708 if (cops->init_decompress_ctx) {
709 ret = cops->init_decompress_ctx(dic);
710 if (ret)
711 goto out_free_dic;
712 }
713
714 dic->rbuf = vmap(dic->tpages, dic->cluster_size, VM_MAP, PAGE_KERNEL);
715 if (!dic->rbuf) {
716 ret = -ENOMEM;
717 goto destroy_decompress_ctx;
718 }
719
720 dic->cbuf = vmap(dic->cpages, dic->nr_cpages, VM_MAP, PAGE_KERNEL_RO);
721 if (!dic->cbuf) {
722 ret = -ENOMEM;
723 goto out_vunmap_rbuf;
724 }
725
726 dic->clen = le32_to_cpu(dic->cbuf->clen);
727 dic->rlen = PAGE_SIZE << dic->log_cluster_size;
728
729 if (dic->clen > PAGE_SIZE * dic->nr_cpages - COMPRESS_HEADER_SIZE) {
730 ret = -EFSCORRUPTED;
731 goto out_vunmap_cbuf;
732 }
733
734 ret = cops->decompress_pages(dic);
735
736 out_vunmap_cbuf:
737 vunmap(dic->cbuf);
738 out_vunmap_rbuf:
739 vunmap(dic->rbuf);
740 destroy_decompress_ctx:
741 if (cops->destroy_decompress_ctx)
742 cops->destroy_decompress_ctx(dic);
743 out_free_dic:
744 if (verity)
745 refcount_set(&dic->ref, dic->nr_cpages);
746 if (!verity)
747 f2fs_decompress_end_io(dic->rpages, dic->cluster_size,
748 ret, false);
749
750 trace_f2fs_decompress_pages_end(dic->inode, dic->cluster_idx,
751 dic->clen, ret);
752 if (!verity)
753 f2fs_free_dic(dic);
754 }
755

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.63 kB)
.config.gz (73.54 kB)
Download all attachments