Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression

From: Chao Yu
Date: Thu Dec 12 2019 - 10:07:48 EST

Next message: Srikar Dronamraju: "Re: [PATCH v4] sched/core: Preempt current task in favour of bound kthread"
Previous message: Dietmar Eggemann: "Re: [PATCH v3 0/5] sched/fair: Task placement biasing using uclamp"
In reply to: Jaegeuk Kim: "Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jaegeuk,

On 2019-12-11 9:27, Jaegeuk Kim wrote:

Hi Chao,

Let me know, if it's okay to integrate compression patch all together.
I don't have a critical bug to fix w/ them now.

Cool, let me send a new RFC with below fix applied.

Thanks,

Another fix:
---
fs/f2fs/compress.c | 101 ++++++++++++++++++++++++++++-----------------
fs/f2fs/data.c | 15 ++++---
fs/f2fs/f2fs.h | 1 -
3 files changed, 72 insertions(+), 45 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 7ebd2bc018bd..af23ed6deffd 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -73,6 +73,17 @@ static void f2fs_put_compressed_page(struct page *page)
put_page(page);
}

+static void f2fs_put_rpages(struct compress_ctx *cc)
+{
+ unsigned int i;
+
+ for (i = 0; i < cc->cluster_size; i++) {
+ if (!cc->rpages[i])
+ continue;
+ put_page(cc->rpages[i]);
+ }
+}
+
struct page *f2fs_compress_control_page(struct page *page)
{
return ((struct compress_io_ctx *)page_private(page))->rpages[0];
@@ -93,7 +104,10 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
void f2fs_destroy_compress_ctx(struct compress_ctx *cc)
{
kfree(cc->rpages);
- f2fs_reset_compress_ctx(cc);
+ cc->rpages = NULL;
+ cc->nr_rpages = 0;
+ cc->nr_cpages = 0;
+ cc->cluster_idx = NULL_CLUSTER;
}

void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page)
@@ -536,14 +550,6 @@ static bool cluster_may_compress(struct compress_ctx *cc)
return __cluster_may_compress(cc);
}

-void f2fs_reset_compress_ctx(struct compress_ctx *cc)
-{
- cc->rpages = NULL;
- cc->nr_rpages = 0;
- cc->nr_cpages = 0;
- cc->cluster_idx = NULL_CLUSTER;
-}
-
static void set_cluster_writeback(struct compress_ctx *cc)
{
int i;
@@ -602,13 +608,13 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
ret = f2fs_read_multi_pages(cc, &bio, cc->cluster_size,
&last_block_in_bio, false);
if (ret)
- return ret;
+ goto release_pages;
if (bio)
f2fs_submit_bio(sbi, bio, DATA);

ret = f2fs_init_compress_ctx(cc);
if (ret)
- return ret;
+ goto release_pages;
}

for (i = 0; i < cc->cluster_size; i++) {
@@ -638,9 +644,11 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,

for (i = cc->cluster_size - 1; i > 0; i--) {
ret = f2fs_get_block(&dn, start_idx + i);
- if (ret)
+ if (ret) {
/* TODO: release preallocate blocks */
- goto release_pages;
+ i = cc->cluster_size;
+ goto unlock_pages;
+ }

if (dn.data_blkaddr != NEW_ADDR)
break;
@@ -769,7 +777,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
cic->magic = F2FS_COMPRESSED_PAGE_MAGIC;
cic->inode = inode;
refcount_set(&cic->ref, 1);
- cic->rpages = cc->rpages;
+ cic->rpages = f2fs_kzalloc(sbi, sizeof(struct page *) <<
+ cc->log_cluster_size, GFP_NOFS);
+ if (!cic->rpages)
+ goto out_put_cic;
+
cic->nr_rpages = cc->cluster_size;

for (i = 0; i < cc->nr_cpages; i++) {
@@ -793,7 +805,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,

blkaddr = datablock_addr(dn.inode, dn.node_page,
dn.ofs_in_node);
- fio.page = cc->rpages[i];
+ fio.page = cic->rpages[i] = cc->rpages[i];
fio.old_blkaddr = blkaddr;

/* cluster header */
@@ -819,7 +831,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,

f2fs_bug_on(fio.sbi, blkaddr == NULL_ADDR);

-
if (fio.encrypted)
fio.encrypted_page = cc->cpages[i - 1];
else if (fio.compressed)
@@ -859,17 +870,22 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
fi->last_disk_size = psize;
up_write(&fi->i_sem);
}
- f2fs_reset_compress_ctx(cc);
+ f2fs_put_rpages(cc);
+ f2fs_destroy_compress_ctx(cc);
return 0;

out_destroy_crypt:
- for (i -= 1; i >= 0; i--)
+ kfree(cic->rpages);
+
+ for (--i; i >= 0; i--)
fscrypt_finalize_bounce_page(&cc->cpages[i]);
for (i = 0; i < cc->nr_cpages; i++) {
if (!cc->cpages[i])
continue;
f2fs_put_page(cc->cpages[i], 1);
}
+out_put_cic:
+ kfree(cic);
out_put_dnode:
f2fs_put_dnode(&dn);
out_unlock_op:
@@ -963,37 +979,39 @@ int f2fs_write_multi_pages(struct compress_ctx *cc,
struct f2fs_inode_info *fi = F2FS_I(cc->inode);
const struct f2fs_compress_ops *cops =
f2fs_cops[fi->i_compress_algorithm];
- int err = -EAGAIN;
+ bool compressed = false;
+ int err;

*submitted = 0;
if (cluster_may_compress(cc)) {
err = f2fs_compress_pages(cc);
- if (err) {
- err = -EAGAIN;
+ if (err == -EAGAIN)
goto write;
- }
+ else if (err)
+ goto put_out;
+
err = f2fs_write_compressed_pages(cc, submitted,
wbc, io_type);
cops->destroy_compress_ctx(cc);
+ if (!err)
+ return 0;
+ f2fs_bug_on(F2FS_I_SB(cc->inode), err != -EAGAIN);
}
write:
- if (err == -EAGAIN) {
- bool compressed = false;
-
- f2fs_bug_on(F2FS_I_SB(cc->inode), *submitted);
+ f2fs_bug_on(F2FS_I_SB(cc->inode), *submitted);

- if (is_compressed_cluster(cc))
- compressed = true;
+ if (is_compressed_cluster(cc))
+ compressed = true;

- err = f2fs_write_raw_pages(cc, submitted, wbc,
- io_type, compressed);
- if (compressed) {
- stat_sub_compr_blocks(cc->inode, *submitted);
- F2FS_I(cc->inode)->i_compressed_blocks -= *submitted;
- f2fs_mark_inode_dirty_sync(cc->inode, true);
- }
- f2fs_destroy_compress_ctx(cc);
+ err = f2fs_write_raw_pages(cc, submitted, wbc, io_type, compressed);
+ if (compressed) {
+ stat_sub_compr_blocks(cc->inode, *submitted);
+ F2FS_I(cc->inode)->i_compressed_blocks -= *submitted;
+ f2fs_mark_inode_dirty_sync(cc->inode, true);
}
+put_out:
+ f2fs_put_rpages(cc);
+ f2fs_destroy_compress_ctx(cc);
return err;
}

@@ -1055,7 +1073,13 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc)
dic->tpages[i] = cc->rpages[i];
}

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

@@ -1072,8 +1096,7 @@ void f2fs_free_dic(struct decompress_io_ctx *dic)
for (i = 0; i < dic->cluster_size; i++) {
if (dic->rpages[i])
continue;
- unlock_page(dic->tpages[i]);
- put_page(dic->tpages[i]);
+ f2fs_put_page(dic->tpages[i], 1);
}
kfree(dic->tpages);
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7046b222e8de..19cd03450066 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2099,7 +2099,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
false);
f2fs_free_dic(dic);
f2fs_put_dnode(&dn);
- f2fs_reset_compress_ctx(cc);
+ f2fs_destroy_compress_ctx(cc);
*bio_ret = bio;
return ret;
}
@@ -2117,7 +2117,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,

f2fs_put_dnode(&dn);

- f2fs_reset_compress_ctx(cc);
+ f2fs_destroy_compress_ctx(cc);
*bio_ret = bio;
return 0;

@@ -2125,7 +2125,6 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
f2fs_put_dnode(&dn);
out:
f2fs_decompress_end_io(cc->rpages, cc->cluster_size, true, false);
- f2fs_destroy_compress_ctx(cc);
*bio_ret = bio;
return ret;
}
@@ -2192,8 +2191,10 @@ int f2fs_mpage_readpages(struct address_space *mapping,
max_nr_pages,
&last_block_in_bio,
is_readahead);
- if (ret)
+ if (ret) {
+ f2fs_destroy_compress_ctx(&cc);
goto set_error_page;
+ }
}
ret = f2fs_is_compressed_cluster(inode, page->index);
if (ret < 0)
@@ -2229,11 +2230,14 @@ int f2fs_mpage_readpages(struct address_space *mapping,
#ifdef CONFIG_F2FS_FS_COMPRESSION
if (f2fs_compressed_file(inode)) {
/* last page */
- if (nr_pages == 1 && !f2fs_cluster_is_empty(&cc))
+ if (nr_pages == 1 && !f2fs_cluster_is_empty(&cc)) {
ret = f2fs_read_multi_pages(&cc, &bio,
max_nr_pages,
&last_block_in_bio,
is_readahead);
+ if (ret)
+ f2fs_destroy_compress_ctx(&cc);
+ }
}
#endif
}
@@ -2856,6 +2860,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,

#ifdef CONFIG_F2FS_FS_COMPRESSION
if (f2fs_compressed_file(inode)) {
+ get_page(page);
f2fs_compress_ctx_add_page(&cc, page);
continue;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 26a4cc1fd686..5d55cef66410 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3765,7 +3765,6 @@ static inline bool f2fs_post_read_required(struct inode *inode)
#ifdef CONFIG_F2FS_FS_COMPRESSION
bool f2fs_is_compressed_page(struct page *page);
struct page *f2fs_compress_control_page(struct page *page);
-void f2fs_reset_compress_ctx(struct compress_ctx *cc);
int f2fs_prepare_compress_overwrite(struct inode *inode,
struct page **pagep, pgoff_t index, void **fsdata);
bool f2fs_compress_write_end(struct inode *inode, void *fsdata,

Next message: Srikar Dronamraju: "Re: [PATCH v4] sched/core: Preempt current task in favour of bound kthread"
Previous message: Dietmar Eggemann: "Re: [PATCH v3 0/5] sched/fair: Task placement biasing using uclamp"
In reply to: Jaegeuk Kim: "Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]