2021-01-07 09:36:04

by Chao Yu

[permalink] [raw]
Subject: [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

Support to use address space of inner inode to cache compressed block,
in order to improve cache hit ratio of random read.

Signed-off-by: Chao Yu <[email protected]>
---
v3:
- rebase to last dev branch.
- add blkaddr sanity check in f2fs_cache_compressed_page()
Documentation/filesystems/f2fs.rst | 3 +
fs/f2fs/compress.c | 171 ++++++++++++++++++++++++++++-
fs/f2fs/data.c | 19 +++-
fs/f2fs/debug.c | 13 +++
fs/f2fs/f2fs.h | 39 ++++++-
fs/f2fs/gc.c | 1 +
fs/f2fs/inode.c | 21 +++-
fs/f2fs/segment.c | 6 +-
fs/f2fs/super.c | 19 +++-
include/linux/f2fs_fs.h | 1 +
10 files changed, 282 insertions(+), 11 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 5eff4009e77e..cd1e5b826ba3 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -273,6 +273,9 @@ compress_mode=%s Control file compression mode. This supports "fs" and "user"
choosing the target file and the timing. The user can do manual
compression/decompression on the compression enabled files using
ioctls.
+compress_cache Support to use address space of a filesystem managed inode to
+ cache compressed block, in order to improve cache hit ratio of
+ random read.
inlinecrypt When possible, encrypt/decrypt the contents of encrypted
files using the blk-crypto framework rather than
filesystem-layer encryption. This allows the use of
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 1696f9183ff5..cb16b0437bd4 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -12,9 +12,11 @@
#include <linux/lzo.h>
#include <linux/lz4.h>
#include <linux/zstd.h>
+#include <linux/pagevec.h>

#include "f2fs.h"
#include "node.h"
+#include "segment.h"
#include <trace/events/f2fs.h>

static struct kmem_cache *cic_entry_slab;
@@ -756,7 +758,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
return ret;
}

-static void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
+void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(dic->inode);
struct f2fs_inode_info *fi = F2FS_I(dic->inode);
@@ -855,7 +857,8 @@ static void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
* page being waited on in the cluster, and if so, it decompresses the cluster
* (or in the case of a failure, cleans up without actually decompressing).
*/
-void f2fs_end_read_compressed_page(struct page *page, bool failed)
+void f2fs_end_read_compressed_page(struct page *page, bool failed,
+ block_t blkaddr)
{
struct decompress_io_ctx *dic =
(struct decompress_io_ctx *)page_private(page);
@@ -865,6 +868,9 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed)

if (failed)
WRITE_ONCE(dic->failed, true);
+ else if (blkaddr)
+ f2fs_cache_compressed_page(sbi, page,
+ dic->inode->i_ino, blkaddr);

if (atomic_dec_and_test(&dic->remaining_pages))
f2fs_decompress_cluster(dic);
@@ -1702,6 +1708,167 @@ void f2fs_put_page_dic(struct page *page)
f2fs_put_dic(dic);
}

+const struct address_space_operations f2fs_compress_aops = {
+ .releasepage = f2fs_release_page,
+ .invalidatepage = f2fs_invalidate_page,
+};
+
+struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi)
+{
+ return sbi->compress_inode->i_mapping;
+}
+
+void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr)
+{
+ if (!sbi->compress_inode)
+ return;
+ invalidate_mapping_pages(COMPRESS_MAPPING(sbi), blkaddr, blkaddr);
+}
+
+void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+ nid_t ino, block_t blkaddr)
+{
+ struct page *cpage;
+ int ret;
+ struct sysinfo si;
+ unsigned long free_ram, avail_ram;
+
+ if (!test_opt(sbi, COMPRESS_CACHE))
+ return;
+
+ if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
+ return;
+
+ si_meminfo(&si);
+ free_ram = si.freeram;
+ avail_ram = si.totalram - si.totalhigh;
+
+ /* free memory is lower than watermark, deny caching compress page */
+ if (free_ram <= sbi->compress_watermark / 100 * avail_ram)
+ return;
+
+ /* cached page count exceed threshold, deny caching compress page */
+ if (COMPRESS_MAPPING(sbi)->nrpages >=
+ free_ram / 100 * sbi->compress_percent)
+ return;
+
+ cpage = find_get_page(COMPRESS_MAPPING(sbi), blkaddr);
+ if (cpage) {
+ f2fs_put_page(cpage, 0);
+ return;
+ }
+
+ cpage = alloc_page(__GFP_IO);
+ if (!cpage)
+ return;
+
+ ret = add_to_page_cache_lru(cpage, COMPRESS_MAPPING(sbi),
+ blkaddr, GFP_NOFS);
+ if (ret) {
+ f2fs_put_page(cpage, 0);
+ return;
+ }
+
+ memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
+ SetPageUptodate(cpage);
+
+ f2fs_set_page_private(cpage, ino);
+
+ f2fs_put_page(cpage, 1);
+}
+
+void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+ block_t blkaddr)
+{
+ struct page *cpage;
+
+ if (!test_opt(sbi, COMPRESS_CACHE))
+ return;
+
+ cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
+ blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
+ if (cpage) {
+ if (PageUptodate(cpage)) {
+ atomic_inc(&sbi->compress_page_hit);
+ memcpy(page_address(page),
+ page_address(cpage), PAGE_SIZE);
+ SetPageUptodate(page);
+ }
+ f2fs_put_page(cpage, 1);
+ }
+}
+
+void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
+{
+ struct address_space *mapping = sbi->compress_inode->i_mapping;
+ struct pagevec pvec;
+ pgoff_t index = 0;
+ pgoff_t end = MAX_BLKADDR(sbi);
+
+ pagevec_init(&pvec);
+
+ do {
+ unsigned int nr_pages;
+ int i;
+
+ nr_pages = pagevec_lookup_range(&pvec, mapping,
+ &index, end - 1);
+ if (!nr_pages)
+ break;
+
+ for (i = 0; i < nr_pages; i++) {
+ struct page *page = pvec.pages[i];
+
+ if (page->index > end)
+ break;
+
+ lock_page(page);
+ if (page->mapping != mapping) {
+ unlock_page(page);
+ continue;
+ }
+
+ if (ino != page_private(page)) {
+ unlock_page(page);
+ continue;
+ }
+
+ generic_error_remove_page(mapping, page);
+ unlock_page(page);
+ }
+ pagevec_release(&pvec);
+ cond_resched();
+ } while (index < end);
+}
+
+int f2fs_init_compress_inode(struct f2fs_sb_info *sbi)
+{
+ struct inode *inode;
+
+ if (!test_opt(sbi, COMPRESS_CACHE))
+ return 0;
+
+ inode = f2fs_iget(sbi->sb, F2FS_COMPRESS_INO(sbi));
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
+ sbi->compress_inode = inode;
+
+ sbi->compress_percent = COMPRESS_PERCENT;
+ sbi->compress_watermark = COMPRESS_WATERMARK;
+
+ atomic_set(&sbi->compress_page_hit, 0);
+
+ return 0;
+}
+
+void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi)
+{
+ if (!sbi->compress_inode)
+ return;
+ iput(sbi->compress_inode);
+ sbi->compress_inode = NULL;
+}
+
int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi)
{
dev_t dev = sbi->sb->s_bdev->bd_dev;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 4d80f00e5e40..427a527eb17e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -153,7 +153,7 @@ static void f2fs_finish_read_bio(struct bio *bio)

if (f2fs_is_compressed_page(page)) {
if (bio->bi_status)
- f2fs_end_read_compressed_page(page, true);
+ f2fs_end_read_compressed_page(page, true, 0);
f2fs_put_page_dic(page);
continue;
}
@@ -249,15 +249,19 @@ static void f2fs_handle_step_decompress(struct bio_post_read_ctx *ctx)
struct bio_vec *bv;
struct bvec_iter_all iter_all;
bool all_compressed = true;
+ block_t blkaddr = SECTOR_TO_BLOCK(ctx->bio->bi_iter.bi_sector);

bio_for_each_segment_all(bv, ctx->bio, iter_all) {
struct page *page = bv->bv_page;

/* PG_error was set if decryption failed. */
if (f2fs_is_compressed_page(page))
- f2fs_end_read_compressed_page(page, PageError(page));
+ f2fs_end_read_compressed_page(page, PageError(page),
+ blkaddr);
else
all_compressed = false;
+
+ blkaddr++;
}

/*
@@ -1385,9 +1389,11 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
old_blkaddr = dn->data_blkaddr;
f2fs_allocate_data_block(sbi, NULL, old_blkaddr, &dn->data_blkaddr,
&sum, seg_type, NULL);
- if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO)
+ if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO) {
invalidate_mapping_pages(META_MAPPING(sbi),
old_blkaddr, old_blkaddr);
+ f2fs_invalidate_compress_page(sbi, old_blkaddr);
+ }
f2fs_update_data_blkaddr(dn, dn->data_blkaddr);

/*
@@ -2205,6 +2211,13 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
blkaddr = data_blkaddr(dn.inode, dn.node_page,
dn.ofs_in_node + i + 1);

+ f2fs_load_compressed_page(sbi, page, blkaddr);
+ if (PageUptodate(page)) {
+ if (atomic_dec_and_test(&dic->remaining_pages))
+ f2fs_decompress_cluster(dic);
+ continue;
+ }
+
if (bio && (!page_is_mergeable(sbi, bio,
*last_block_in_bio, blkaddr) ||
!f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 197c914119da..f1f8714066c5 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -145,6 +145,12 @@ static void update_general_status(struct f2fs_sb_info *sbi)
si->node_pages = NODE_MAPPING(sbi)->nrpages;
if (sbi->meta_inode)
si->meta_pages = META_MAPPING(sbi)->nrpages;
+#ifdef CONFIG_F2FS_FS_COMPRESSION
+ if (sbi->compress_inode) {
+ si->compress_pages = COMPRESS_MAPPING(sbi)->nrpages;
+ si->compress_page_hit = atomic_read(&sbi->compress_page_hit);
+ }
+#endif
si->nats = NM_I(sbi)->nat_cnt[TOTAL_NAT];
si->dirty_nats = NM_I(sbi)->nat_cnt[DIRTY_NAT];
si->sits = MAIN_SEGS(sbi);
@@ -299,6 +305,12 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
unsigned npages = META_MAPPING(sbi)->nrpages;
si->page_mem += (unsigned long long)npages << PAGE_SHIFT;
}
+#ifdef CONFIG_F2FS_FS_COMPRESSION
+ if (sbi->compress_inode) {
+ unsigned npages = COMPRESS_MAPPING(sbi)->nrpages;
+ si->page_mem += (unsigned long long)npages << PAGE_SHIFT;
+ }
+#endif
}

static int stat_show(struct seq_file *s, void *v)
@@ -461,6 +473,7 @@ static int stat_show(struct seq_file *s, void *v)
"volatile IO: %4d (Max. %4d)\n",
si->inmem_pages, si->aw_cnt, si->max_aw_cnt,
si->vw_cnt, si->max_vw_cnt);
+ seq_printf(s, " - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
seq_printf(s, " - nodes: %4d in %4d\n",
si->ndirty_node, si->node_pages);
seq_printf(s, " - dents: %4d in dirs:%4d (%4d)\n",
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 980e061f7968..67086c6072ad 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -97,6 +97,7 @@ extern const char *f2fs_fault_name[FAULT_MAX];
#define F2FS_MOUNT_DISABLE_CHECKPOINT 0x02000000
#define F2FS_MOUNT_NORECOVERY 0x04000000
#define F2FS_MOUNT_ATGC 0x08000000
+#define F2FS_MOUNT_COMPRESS_CACHE 0x10000000

#define F2FS_OPTION(sbi) ((sbi)->mount_opt)
#define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
@@ -1300,6 +1301,9 @@ enum compress_flag {
COMPRESS_MAX_FLAG,
};

+#define COMPRESS_WATERMARK 20
+#define COMPRESS_PERCENT 20
+
#define COMPRESS_DATA_RESERVED_SIZE 4
struct compress_data {
__le32 clen; /* compressed data size */
@@ -1604,6 +1608,11 @@ struct f2fs_sb_info {
#ifdef CONFIG_F2FS_FS_COMPRESSION
struct kmem_cache *page_array_slab; /* page array entry */
unsigned int page_array_slab_size; /* default page array slab size */
+
+ struct inode *compress_inode; /* cache compressed blocks */
+ unsigned int compress_percent; /* cache page percentage */
+ unsigned int compress_watermark; /* cache page watermark */
+ atomic_t compress_page_hit; /* cache hit count */
#endif
};

@@ -3571,7 +3580,8 @@ struct f2fs_stat_info {
unsigned int bimodal, avg_vblocks;
int util_free, util_valid, util_invalid;
int rsvd_segs, overp_segs;
- int dirty_count, node_pages, meta_pages;
+ int dirty_count, node_pages, meta_pages, compress_pages;
+ int compress_page_hit;
int prefree_count, call_count, cp_count, bg_cp_count;
int tot_segs, node_segs, data_segs, free_segs, free_secs;
int bg_node_segs, bg_data_segs;
@@ -3909,7 +3919,9 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
bool f2fs_is_compress_backend_ready(struct inode *inode);
int f2fs_init_compress_mempool(void);
void f2fs_destroy_compress_mempool(void);
-void f2fs_end_read_compressed_page(struct page *page, bool failed);
+void f2fs_decompress_cluster(struct decompress_io_ctx *dic);
+void f2fs_end_read_compressed_page(struct page *page, bool failed,
+ block_t blkaddr);
bool f2fs_cluster_is_empty(struct compress_ctx *cc);
bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page);
@@ -3927,10 +3939,19 @@ void f2fs_put_page_dic(struct page *page);
int f2fs_init_compress_ctx(struct compress_ctx *cc);
void f2fs_destroy_compress_ctx(struct compress_ctx *cc);
void f2fs_init_compress_info(struct f2fs_sb_info *sbi);
+int f2fs_init_compress_inode(struct f2fs_sb_info *sbi);
+void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi);
int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi);
void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi);
int __init f2fs_init_compress_cache(void);
void f2fs_destroy_compress_cache(void);
+struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
+void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
+void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+ nid_t ino, block_t blkaddr);
+void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+ block_t blkaddr);
+void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
#else
static inline bool f2fs_is_compressed_page(struct page *page) { return false; }
static inline bool f2fs_is_compress_backend_ready(struct inode *inode)
@@ -3947,7 +3968,9 @@ static inline struct page *f2fs_compress_control_page(struct page *page)
}
static inline int f2fs_init_compress_mempool(void) { return 0; }
static inline void f2fs_destroy_compress_mempool(void) { }
-static inline void f2fs_end_read_compressed_page(struct page *page, bool failed)
+static inline void f2fs_decompress_cluster(struct decompress_io_ctx *dic) { }
+static inline void f2fs_end_read_compressed_page(struct page *page,
+ bool failed, block_t blkaddr)
{
WARN_ON_ONCE(1);
}
@@ -3955,10 +3978,20 @@ static inline void f2fs_put_page_dic(struct page *page)
{
WARN_ON_ONCE(1);
}
+static inline int f2fs_init_compress_inode(struct f2fs_sb_info *sbi) { return 0; }
+static inline void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi) { }
static inline int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi) { return 0; }
static inline void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi) { }
static inline int __init f2fs_init_compress_cache(void) { return 0; }
static inline void f2fs_destroy_compress_cache(void) { }
+static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
+ block_t blkaddr) { }
+static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
+ struct page *page, nid_t ino, block_t blkaddr) { }
+static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
+ struct page *page, block_t blkaddr) { }
+static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
+ nid_t ino) { }
#endif

static inline void set_compress_context(struct inode *inode)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 39330ad3c44e..4561c44dfa8f 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1226,6 +1226,7 @@ static int move_data_block(struct inode *inode, block_t bidx,
f2fs_put_page(mpage, 1);
invalidate_mapping_pages(META_MAPPING(fio.sbi),
fio.old_blkaddr, fio.old_blkaddr);
+ f2fs_invalidate_compress_page(fio.sbi, fio.old_blkaddr);

set_page_dirty(fio.encrypted_page);
if (clear_page_dirty_for_io(fio.encrypted_page))
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 349d9cb933ee..f030b9b79202 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -18,6 +18,10 @@

#include <trace/events/f2fs.h>

+#ifdef CONFIG_F2FS_FS_COMPRESSION
+extern const struct address_space_operations f2fs_compress_aops;
+#endif
+
void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync)
{
if (is_inode_flag_set(inode, FI_NEW_INODE))
@@ -494,6 +498,11 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
if (ino == F2FS_NODE_INO(sbi) || ino == F2FS_META_INO(sbi))
goto make_now;

+#ifdef CONFIG_F2FS_FS_COMPRESSION
+ if (ino == F2FS_COMPRESS_INO(sbi))
+ goto make_now;
+#endif
+
ret = do_read_inode(inode);
if (ret)
goto bad_inode;
@@ -504,6 +513,12 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
} else if (ino == F2FS_META_INO(sbi)) {
inode->i_mapping->a_ops = &f2fs_meta_aops;
mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS);
+ } else if (ino == F2FS_COMPRESS_INO(sbi)) {
+#ifdef CONFIG_F2FS_FS_COMPRESSION
+ inode->i_mapping->a_ops = &f2fs_compress_aops;
+#endif
+ mapping_set_gfp_mask(inode->i_mapping,
+ GFP_NOFS | __GFP_HIGHMEM | __GFP_MOVABLE);
} else if (S_ISREG(inode->i_mode)) {
inode->i_op = &f2fs_file_inode_operations;
inode->i_fop = &f2fs_file_operations;
@@ -722,8 +737,12 @@ void f2fs_evict_inode(struct inode *inode)
trace_f2fs_evict_inode(inode);
truncate_inode_pages_final(&inode->i_data);

+ if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
+ f2fs_invalidate_compress_pages(sbi, inode->i_ino);
+
if (inode->i_ino == F2FS_NODE_INO(sbi) ||
- inode->i_ino == F2FS_META_INO(sbi))
+ inode->i_ino == F2FS_META_INO(sbi) ||
+ inode->i_ino == F2FS_COMPRESS_INO(sbi))
goto out_clear;

f2fs_bug_on(sbi, get_dirty_pages(inode));
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 11b2bba01353..dab870d9faf6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2312,6 +2312,7 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
return;

invalidate_mapping_pages(META_MAPPING(sbi), addr, addr);
+ f2fs_invalidate_compress_page(sbi, addr);

/* add it into sit main buffer */
down_write(&sit_i->sentry_lock);
@@ -3439,9 +3440,11 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
reallocate:
f2fs_allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
&fio->new_blkaddr, sum, type, fio);
- if (GET_SEGNO(fio->sbi, fio->old_blkaddr) != NULL_SEGNO)
+ if (GET_SEGNO(fio->sbi, fio->old_blkaddr) != NULL_SEGNO) {
invalidate_mapping_pages(META_MAPPING(fio->sbi),
fio->old_blkaddr, fio->old_blkaddr);
+ f2fs_invalidate_compress_page(fio->sbi, fio->old_blkaddr);
+ }

/* writeout dirty page into bdev */
f2fs_submit_page_write(fio);
@@ -3614,6 +3617,7 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO) {
invalidate_mapping_pages(META_MAPPING(sbi),
old_blkaddr, old_blkaddr);
+ f2fs_invalidate_compress_page(sbi, old_blkaddr);
if (!from_gc)
update_segment_mtime(sbi, old_blkaddr, 0);
update_sit_entry(sbi, old_blkaddr, -1);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index c39c03ac7468..18d9a7ece24d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -150,6 +150,7 @@ enum {
Opt_compress_extension,
Opt_compress_chksum,
Opt_compress_mode,
+ Opt_compress_cache,
Opt_atgc,
Opt_err,
};
@@ -220,6 +221,7 @@ static match_table_t f2fs_tokens = {
{Opt_compress_extension, "compress_extension=%s"},
{Opt_compress_chksum, "compress_chksum"},
{Opt_compress_mode, "compress_mode=%s"},
+ {Opt_compress_cache, "compress_cache"},
{Opt_atgc, "atgc"},
{Opt_err, NULL},
};
@@ -1035,12 +1037,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
}
kfree(name);
break;
+ case Opt_compress_cache:
+ set_opt(sbi, COMPRESS_CACHE);
+ break;
#else
case Opt_compress_algorithm:
case Opt_compress_log_size:
case Opt_compress_extension:
case Opt_compress_chksum:
case Opt_compress_mode:
+ case Opt_compress_cache:
f2fs_info(sbi, "compression options not supported");
break;
#endif
@@ -1365,6 +1371,8 @@ static void f2fs_put_super(struct super_block *sb)

f2fs_bug_on(sbi, sbi->fsync_node_num);

+ f2fs_destroy_compress_inode(sbi);
+
iput(sbi->node_inode);
sbi->node_inode = NULL;

@@ -1637,6 +1645,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
seq_printf(seq, ",compress_mode=%s", "fs");
else if (F2FS_OPTION(sbi).compress_mode == COMPR_MODE_USER)
seq_printf(seq, ",compress_mode=%s", "user");
+
+ if (test_opt(sbi, COMPRESS_CACHE))
+ seq_puts(seq, ",compress_cache");
}

static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
@@ -3841,10 +3852,14 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
goto free_node_inode;
}

- err = f2fs_register_sysfs(sbi);
+ err = f2fs_init_compress_inode(sbi);
if (err)
goto free_root_inode;

+ err = f2fs_register_sysfs(sbi);
+ if (err)
+ goto free_compress_inode;
+
#ifdef CONFIG_QUOTA
/* Enable quota usage during mount */
if (f2fs_sb_has_quota_ino(sbi) && !f2fs_readonly(sb)) {
@@ -3978,6 +3993,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
/* evict some inodes being cached by GC */
evict_inodes(sb);
f2fs_unregister_sysfs(sbi);
+free_compress_inode:
+ f2fs_destroy_compress_inode(sbi);
free_root_inode:
dput(sb->s_root);
sb->s_root = NULL;
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index c6cc0a566ef5..2dcc63fe8494 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -34,6 +34,7 @@
#define F2FS_ROOT_INO(sbi) ((sbi)->root_ino_num)
#define F2FS_NODE_INO(sbi) ((sbi)->node_ino_num)
#define F2FS_META_INO(sbi) ((sbi)->meta_ino_num)
+#define F2FS_COMPRESS_INO(sbi) (NM_I(sbi)->max_nid)

#define F2FS_MAX_QUOTAS 3

--
2.29.2


2021-01-11 09:52:32

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

Hi Chao,

After quick test of fsstress w/ fault injection, it gave wrong block address
errors. Could you please run the test a bit?

Thanks,

On 01/07, Chao Yu wrote:
> Support to use address space of inner inode to cache compressed block,
> in order to improve cache hit ratio of random read.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> v3:
> - rebase to last dev branch.
> - add blkaddr sanity check in f2fs_cache_compressed_page()
> Documentation/filesystems/f2fs.rst | 3 +
> fs/f2fs/compress.c | 171 ++++++++++++++++++++++++++++-
> fs/f2fs/data.c | 19 +++-
> fs/f2fs/debug.c | 13 +++
> fs/f2fs/f2fs.h | 39 ++++++-
> fs/f2fs/gc.c | 1 +
> fs/f2fs/inode.c | 21 +++-
> fs/f2fs/segment.c | 6 +-
> fs/f2fs/super.c | 19 +++-
> include/linux/f2fs_fs.h | 1 +
> 10 files changed, 282 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 5eff4009e77e..cd1e5b826ba3 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -273,6 +273,9 @@ compress_mode=%s Control file compression mode. This supports "fs" and "user"
> choosing the target file and the timing. The user can do manual
> compression/decompression on the compression enabled files using
> ioctls.
> +compress_cache Support to use address space of a filesystem managed inode to
> + cache compressed block, in order to improve cache hit ratio of
> + random read.
> inlinecrypt When possible, encrypt/decrypt the contents of encrypted
> files using the blk-crypto framework rather than
> filesystem-layer encryption. This allows the use of
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 1696f9183ff5..cb16b0437bd4 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -12,9 +12,11 @@
> #include <linux/lzo.h>
> #include <linux/lz4.h>
> #include <linux/zstd.h>
> +#include <linux/pagevec.h>
>
> #include "f2fs.h"
> #include "node.h"
> +#include "segment.h"
> #include <trace/events/f2fs.h>
>
> static struct kmem_cache *cic_entry_slab;
> @@ -756,7 +758,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> return ret;
> }
>
> -static void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
> +void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(dic->inode);
> struct f2fs_inode_info *fi = F2FS_I(dic->inode);
> @@ -855,7 +857,8 @@ static void f2fs_decompress_cluster(struct decompress_io_ctx *dic)
> * page being waited on in the cluster, and if so, it decompresses the cluster
> * (or in the case of a failure, cleans up without actually decompressing).
> */
> -void f2fs_end_read_compressed_page(struct page *page, bool failed)
> +void f2fs_end_read_compressed_page(struct page *page, bool failed,
> + block_t blkaddr)
> {
> struct decompress_io_ctx *dic =
> (struct decompress_io_ctx *)page_private(page);
> @@ -865,6 +868,9 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed)
>
> if (failed)
> WRITE_ONCE(dic->failed, true);
> + else if (blkaddr)
> + f2fs_cache_compressed_page(sbi, page,
> + dic->inode->i_ino, blkaddr);
>
> if (atomic_dec_and_test(&dic->remaining_pages))
> f2fs_decompress_cluster(dic);
> @@ -1702,6 +1708,167 @@ void f2fs_put_page_dic(struct page *page)
> f2fs_put_dic(dic);
> }
>
> +const struct address_space_operations f2fs_compress_aops = {
> + .releasepage = f2fs_release_page,
> + .invalidatepage = f2fs_invalidate_page,
> +};
> +
> +struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi)
> +{
> + return sbi->compress_inode->i_mapping;
> +}
> +
> +void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr)
> +{
> + if (!sbi->compress_inode)
> + return;
> + invalidate_mapping_pages(COMPRESS_MAPPING(sbi), blkaddr, blkaddr);
> +}
> +
> +void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> + nid_t ino, block_t blkaddr)
> +{
> + struct page *cpage;
> + int ret;
> + struct sysinfo si;
> + unsigned long free_ram, avail_ram;
> +
> + if (!test_opt(sbi, COMPRESS_CACHE))
> + return;
> +
> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> + return;
> +
> + si_meminfo(&si);
> + free_ram = si.freeram;
> + avail_ram = si.totalram - si.totalhigh;
> +
> + /* free memory is lower than watermark, deny caching compress page */
> + if (free_ram <= sbi->compress_watermark / 100 * avail_ram)
> + return;
> +
> + /* cached page count exceed threshold, deny caching compress page */
> + if (COMPRESS_MAPPING(sbi)->nrpages >=
> + free_ram / 100 * sbi->compress_percent)
> + return;
> +
> + cpage = find_get_page(COMPRESS_MAPPING(sbi), blkaddr);
> + if (cpage) {
> + f2fs_put_page(cpage, 0);
> + return;
> + }
> +
> + cpage = alloc_page(__GFP_IO);
> + if (!cpage)
> + return;
> +
> + ret = add_to_page_cache_lru(cpage, COMPRESS_MAPPING(sbi),
> + blkaddr, GFP_NOFS);
> + if (ret) {
> + f2fs_put_page(cpage, 0);
> + return;
> + }
> +
> + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> + SetPageUptodate(cpage);
> +
> + f2fs_set_page_private(cpage, ino);
> +
> + f2fs_put_page(cpage, 1);
> +}
> +
> +void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> + block_t blkaddr)
> +{
> + struct page *cpage;
> +
> + if (!test_opt(sbi, COMPRESS_CACHE))
> + return;
> +
> + cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> + blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> + if (cpage) {
> + if (PageUptodate(cpage)) {
> + atomic_inc(&sbi->compress_page_hit);
> + memcpy(page_address(page),
> + page_address(cpage), PAGE_SIZE);
> + SetPageUptodate(page);
> + }
> + f2fs_put_page(cpage, 1);
> + }
> +}
> +
> +void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> +{
> + struct address_space *mapping = sbi->compress_inode->i_mapping;
> + struct pagevec pvec;
> + pgoff_t index = 0;
> + pgoff_t end = MAX_BLKADDR(sbi);
> +
> + pagevec_init(&pvec);
> +
> + do {
> + unsigned int nr_pages;
> + int i;
> +
> + nr_pages = pagevec_lookup_range(&pvec, mapping,
> + &index, end - 1);
> + if (!nr_pages)
> + break;
> +
> + for (i = 0; i < nr_pages; i++) {
> + struct page *page = pvec.pages[i];
> +
> + if (page->index > end)
> + break;
> +
> + lock_page(page);
> + if (page->mapping != mapping) {
> + unlock_page(page);
> + continue;
> + }
> +
> + if (ino != page_private(page)) {
> + unlock_page(page);
> + continue;
> + }
> +
> + generic_error_remove_page(mapping, page);
> + unlock_page(page);
> + }
> + pagevec_release(&pvec);
> + cond_resched();
> + } while (index < end);
> +}
> +
> +int f2fs_init_compress_inode(struct f2fs_sb_info *sbi)
> +{
> + struct inode *inode;
> +
> + if (!test_opt(sbi, COMPRESS_CACHE))
> + return 0;
> +
> + inode = f2fs_iget(sbi->sb, F2FS_COMPRESS_INO(sbi));
> + if (IS_ERR(inode))
> + return PTR_ERR(inode);
> + sbi->compress_inode = inode;
> +
> + sbi->compress_percent = COMPRESS_PERCENT;
> + sbi->compress_watermark = COMPRESS_WATERMARK;
> +
> + atomic_set(&sbi->compress_page_hit, 0);
> +
> + return 0;
> +}
> +
> +void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi)
> +{
> + if (!sbi->compress_inode)
> + return;
> + iput(sbi->compress_inode);
> + sbi->compress_inode = NULL;
> +}
> +
> int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi)
> {
> dev_t dev = sbi->sb->s_bdev->bd_dev;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 4d80f00e5e40..427a527eb17e 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -153,7 +153,7 @@ static void f2fs_finish_read_bio(struct bio *bio)
>
> if (f2fs_is_compressed_page(page)) {
> if (bio->bi_status)
> - f2fs_end_read_compressed_page(page, true);
> + f2fs_end_read_compressed_page(page, true, 0);
> f2fs_put_page_dic(page);
> continue;
> }
> @@ -249,15 +249,19 @@ static void f2fs_handle_step_decompress(struct bio_post_read_ctx *ctx)
> struct bio_vec *bv;
> struct bvec_iter_all iter_all;
> bool all_compressed = true;
> + block_t blkaddr = SECTOR_TO_BLOCK(ctx->bio->bi_iter.bi_sector);
>
> bio_for_each_segment_all(bv, ctx->bio, iter_all) {
> struct page *page = bv->bv_page;
>
> /* PG_error was set if decryption failed. */
> if (f2fs_is_compressed_page(page))
> - f2fs_end_read_compressed_page(page, PageError(page));
> + f2fs_end_read_compressed_page(page, PageError(page),
> + blkaddr);
> else
> all_compressed = false;
> +
> + blkaddr++;
> }
>
> /*
> @@ -1385,9 +1389,11 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
> old_blkaddr = dn->data_blkaddr;
> f2fs_allocate_data_block(sbi, NULL, old_blkaddr, &dn->data_blkaddr,
> &sum, seg_type, NULL);
> - if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO)
> + if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO) {
> invalidate_mapping_pages(META_MAPPING(sbi),
> old_blkaddr, old_blkaddr);
> + f2fs_invalidate_compress_page(sbi, old_blkaddr);
> + }
> f2fs_update_data_blkaddr(dn, dn->data_blkaddr);
>
> /*
> @@ -2205,6 +2211,13 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> blkaddr = data_blkaddr(dn.inode, dn.node_page,
> dn.ofs_in_node + i + 1);
>
> + f2fs_load_compressed_page(sbi, page, blkaddr);
> + if (PageUptodate(page)) {
> + if (atomic_dec_and_test(&dic->remaining_pages))
> + f2fs_decompress_cluster(dic);
> + continue;
> + }
> +
> if (bio && (!page_is_mergeable(sbi, bio,
> *last_block_in_bio, blkaddr) ||
> !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 197c914119da..f1f8714066c5 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -145,6 +145,12 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> si->node_pages = NODE_MAPPING(sbi)->nrpages;
> if (sbi->meta_inode)
> si->meta_pages = META_MAPPING(sbi)->nrpages;
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> + if (sbi->compress_inode) {
> + si->compress_pages = COMPRESS_MAPPING(sbi)->nrpages;
> + si->compress_page_hit = atomic_read(&sbi->compress_page_hit);
> + }
> +#endif
> si->nats = NM_I(sbi)->nat_cnt[TOTAL_NAT];
> si->dirty_nats = NM_I(sbi)->nat_cnt[DIRTY_NAT];
> si->sits = MAIN_SEGS(sbi);
> @@ -299,6 +305,12 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
> unsigned npages = META_MAPPING(sbi)->nrpages;
> si->page_mem += (unsigned long long)npages << PAGE_SHIFT;
> }
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> + if (sbi->compress_inode) {
> + unsigned npages = COMPRESS_MAPPING(sbi)->nrpages;
> + si->page_mem += (unsigned long long)npages << PAGE_SHIFT;
> + }
> +#endif
> }
>
> static int stat_show(struct seq_file *s, void *v)
> @@ -461,6 +473,7 @@ static int stat_show(struct seq_file *s, void *v)
> "volatile IO: %4d (Max. %4d)\n",
> si->inmem_pages, si->aw_cnt, si->max_aw_cnt,
> si->vw_cnt, si->max_vw_cnt);
> + seq_printf(s, " - compress: %4d, hit:%8d\n", si->compress_pages, si->compress_page_hit);
> seq_printf(s, " - nodes: %4d in %4d\n",
> si->ndirty_node, si->node_pages);
> seq_printf(s, " - dents: %4d in dirs:%4d (%4d)\n",
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 980e061f7968..67086c6072ad 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -97,6 +97,7 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> #define F2FS_MOUNT_DISABLE_CHECKPOINT 0x02000000
> #define F2FS_MOUNT_NORECOVERY 0x04000000
> #define F2FS_MOUNT_ATGC 0x08000000
> +#define F2FS_MOUNT_COMPRESS_CACHE 0x10000000
>
> #define F2FS_OPTION(sbi) ((sbi)->mount_opt)
> #define clear_opt(sbi, option) (F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
> @@ -1300,6 +1301,9 @@ enum compress_flag {
> COMPRESS_MAX_FLAG,
> };
>
> +#define COMPRESS_WATERMARK 20
> +#define COMPRESS_PERCENT 20
> +
> #define COMPRESS_DATA_RESERVED_SIZE 4
> struct compress_data {
> __le32 clen; /* compressed data size */
> @@ -1604,6 +1608,11 @@ struct f2fs_sb_info {
> #ifdef CONFIG_F2FS_FS_COMPRESSION
> struct kmem_cache *page_array_slab; /* page array entry */
> unsigned int page_array_slab_size; /* default page array slab size */
> +
> + struct inode *compress_inode; /* cache compressed blocks */
> + unsigned int compress_percent; /* cache page percentage */
> + unsigned int compress_watermark; /* cache page watermark */
> + atomic_t compress_page_hit; /* cache hit count */
> #endif
> };
>
> @@ -3571,7 +3580,8 @@ struct f2fs_stat_info {
> unsigned int bimodal, avg_vblocks;
> int util_free, util_valid, util_invalid;
> int rsvd_segs, overp_segs;
> - int dirty_count, node_pages, meta_pages;
> + int dirty_count, node_pages, meta_pages, compress_pages;
> + int compress_page_hit;
> int prefree_count, call_count, cp_count, bg_cp_count;
> int tot_segs, node_segs, data_segs, free_segs, free_secs;
> int bg_node_segs, bg_data_segs;
> @@ -3909,7 +3919,9 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
> bool f2fs_is_compress_backend_ready(struct inode *inode);
> int f2fs_init_compress_mempool(void);
> void f2fs_destroy_compress_mempool(void);
> -void f2fs_end_read_compressed_page(struct page *page, bool failed);
> +void f2fs_decompress_cluster(struct decompress_io_ctx *dic);
> +void f2fs_end_read_compressed_page(struct page *page, bool failed,
> + block_t blkaddr);
> bool f2fs_cluster_is_empty(struct compress_ctx *cc);
> bool f2fs_cluster_can_merge_page(struct compress_ctx *cc, pgoff_t index);
> void f2fs_compress_ctx_add_page(struct compress_ctx *cc, struct page *page);
> @@ -3927,10 +3939,19 @@ void f2fs_put_page_dic(struct page *page);
> int f2fs_init_compress_ctx(struct compress_ctx *cc);
> void f2fs_destroy_compress_ctx(struct compress_ctx *cc);
> void f2fs_init_compress_info(struct f2fs_sb_info *sbi);
> +int f2fs_init_compress_inode(struct f2fs_sb_info *sbi);
> +void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi);
> int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi);
> void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi);
> int __init f2fs_init_compress_cache(void);
> void f2fs_destroy_compress_cache(void);
> +struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> +void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
> +void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> + nid_t ino, block_t blkaddr);
> +void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> + block_t blkaddr);
> +void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> #else
> static inline bool f2fs_is_compressed_page(struct page *page) { return false; }
> static inline bool f2fs_is_compress_backend_ready(struct inode *inode)
> @@ -3947,7 +3968,9 @@ static inline struct page *f2fs_compress_control_page(struct page *page)
> }
> static inline int f2fs_init_compress_mempool(void) { return 0; }
> static inline void f2fs_destroy_compress_mempool(void) { }
> -static inline void f2fs_end_read_compressed_page(struct page *page, bool failed)
> +static inline void f2fs_decompress_cluster(struct decompress_io_ctx *dic) { }
> +static inline void f2fs_end_read_compressed_page(struct page *page,
> + bool failed, block_t blkaddr)
> {
> WARN_ON_ONCE(1);
> }
> @@ -3955,10 +3978,20 @@ static inline void f2fs_put_page_dic(struct page *page)
> {
> WARN_ON_ONCE(1);
> }
> +static inline int f2fs_init_compress_inode(struct f2fs_sb_info *sbi) { return 0; }
> +static inline void f2fs_destroy_compress_inode(struct f2fs_sb_info *sbi) { }
> static inline int f2fs_init_page_array_cache(struct f2fs_sb_info *sbi) { return 0; }
> static inline void f2fs_destroy_page_array_cache(struct f2fs_sb_info *sbi) { }
> static inline int __init f2fs_init_compress_cache(void) { return 0; }
> static inline void f2fs_destroy_compress_cache(void) { }
> +static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> + block_t blkaddr) { }
> +static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> + struct page *page, nid_t ino, block_t blkaddr) { }
> +static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> + struct page *page, block_t blkaddr) { }
> +static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> + nid_t ino) { }
> #endif
>
> static inline void set_compress_context(struct inode *inode)
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 39330ad3c44e..4561c44dfa8f 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1226,6 +1226,7 @@ static int move_data_block(struct inode *inode, block_t bidx,
> f2fs_put_page(mpage, 1);
> invalidate_mapping_pages(META_MAPPING(fio.sbi),
> fio.old_blkaddr, fio.old_blkaddr);
> + f2fs_invalidate_compress_page(fio.sbi, fio.old_blkaddr);
>
> set_page_dirty(fio.encrypted_page);
> if (clear_page_dirty_for_io(fio.encrypted_page))
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 349d9cb933ee..f030b9b79202 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -18,6 +18,10 @@
>
> #include <trace/events/f2fs.h>
>
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> +extern const struct address_space_operations f2fs_compress_aops;
> +#endif
> +
> void f2fs_mark_inode_dirty_sync(struct inode *inode, bool sync)
> {
> if (is_inode_flag_set(inode, FI_NEW_INODE))
> @@ -494,6 +498,11 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
> if (ino == F2FS_NODE_INO(sbi) || ino == F2FS_META_INO(sbi))
> goto make_now;
>
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> + if (ino == F2FS_COMPRESS_INO(sbi))
> + goto make_now;
> +#endif
> +
> ret = do_read_inode(inode);
> if (ret)
> goto bad_inode;
> @@ -504,6 +513,12 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
> } else if (ino == F2FS_META_INO(sbi)) {
> inode->i_mapping->a_ops = &f2fs_meta_aops;
> mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS);
> + } else if (ino == F2FS_COMPRESS_INO(sbi)) {
> +#ifdef CONFIG_F2FS_FS_COMPRESSION
> + inode->i_mapping->a_ops = &f2fs_compress_aops;
> +#endif
> + mapping_set_gfp_mask(inode->i_mapping,
> + GFP_NOFS | __GFP_HIGHMEM | __GFP_MOVABLE);
> } else if (S_ISREG(inode->i_mode)) {
> inode->i_op = &f2fs_file_inode_operations;
> inode->i_fop = &f2fs_file_operations;
> @@ -722,8 +737,12 @@ void f2fs_evict_inode(struct inode *inode)
> trace_f2fs_evict_inode(inode);
> truncate_inode_pages_final(&inode->i_data);
>
> + if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
> + f2fs_invalidate_compress_pages(sbi, inode->i_ino);
> +
> if (inode->i_ino == F2FS_NODE_INO(sbi) ||
> - inode->i_ino == F2FS_META_INO(sbi))
> + inode->i_ino == F2FS_META_INO(sbi) ||
> + inode->i_ino == F2FS_COMPRESS_INO(sbi))
> goto out_clear;
>
> f2fs_bug_on(sbi, get_dirty_pages(inode));
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 11b2bba01353..dab870d9faf6 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2312,6 +2312,7 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
> return;
>
> invalidate_mapping_pages(META_MAPPING(sbi), addr, addr);
> + f2fs_invalidate_compress_page(sbi, addr);
>
> /* add it into sit main buffer */
> down_write(&sit_i->sentry_lock);
> @@ -3439,9 +3440,11 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
> reallocate:
> f2fs_allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
> &fio->new_blkaddr, sum, type, fio);
> - if (GET_SEGNO(fio->sbi, fio->old_blkaddr) != NULL_SEGNO)
> + if (GET_SEGNO(fio->sbi, fio->old_blkaddr) != NULL_SEGNO) {
> invalidate_mapping_pages(META_MAPPING(fio->sbi),
> fio->old_blkaddr, fio->old_blkaddr);
> + f2fs_invalidate_compress_page(fio->sbi, fio->old_blkaddr);
> + }
>
> /* writeout dirty page into bdev */
> f2fs_submit_page_write(fio);
> @@ -3614,6 +3617,7 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO) {
> invalidate_mapping_pages(META_MAPPING(sbi),
> old_blkaddr, old_blkaddr);
> + f2fs_invalidate_compress_page(sbi, old_blkaddr);
> if (!from_gc)
> update_segment_mtime(sbi, old_blkaddr, 0);
> update_sit_entry(sbi, old_blkaddr, -1);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index c39c03ac7468..18d9a7ece24d 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -150,6 +150,7 @@ enum {
> Opt_compress_extension,
> Opt_compress_chksum,
> Opt_compress_mode,
> + Opt_compress_cache,
> Opt_atgc,
> Opt_err,
> };
> @@ -220,6 +221,7 @@ static match_table_t f2fs_tokens = {
> {Opt_compress_extension, "compress_extension=%s"},
> {Opt_compress_chksum, "compress_chksum"},
> {Opt_compress_mode, "compress_mode=%s"},
> + {Opt_compress_cache, "compress_cache"},
> {Opt_atgc, "atgc"},
> {Opt_err, NULL},
> };
> @@ -1035,12 +1037,16 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> }
> kfree(name);
> break;
> + case Opt_compress_cache:
> + set_opt(sbi, COMPRESS_CACHE);
> + break;
> #else
> case Opt_compress_algorithm:
> case Opt_compress_log_size:
> case Opt_compress_extension:
> case Opt_compress_chksum:
> case Opt_compress_mode:
> + case Opt_compress_cache:
> f2fs_info(sbi, "compression options not supported");
> break;
> #endif
> @@ -1365,6 +1371,8 @@ static void f2fs_put_super(struct super_block *sb)
>
> f2fs_bug_on(sbi, sbi->fsync_node_num);
>
> + f2fs_destroy_compress_inode(sbi);
> +
> iput(sbi->node_inode);
> sbi->node_inode = NULL;
>
> @@ -1637,6 +1645,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
> seq_printf(seq, ",compress_mode=%s", "fs");
> else if (F2FS_OPTION(sbi).compress_mode == COMPR_MODE_USER)
> seq_printf(seq, ",compress_mode=%s", "user");
> +
> + if (test_opt(sbi, COMPRESS_CACHE))
> + seq_puts(seq, ",compress_cache");
> }
>
> static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> @@ -3841,10 +3852,14 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> goto free_node_inode;
> }
>
> - err = f2fs_register_sysfs(sbi);
> + err = f2fs_init_compress_inode(sbi);
> if (err)
> goto free_root_inode;
>
> + err = f2fs_register_sysfs(sbi);
> + if (err)
> + goto free_compress_inode;
> +
> #ifdef CONFIG_QUOTA
> /* Enable quota usage during mount */
> if (f2fs_sb_has_quota_ino(sbi) && !f2fs_readonly(sb)) {
> @@ -3978,6 +3993,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> /* evict some inodes being cached by GC */
> evict_inodes(sb);
> f2fs_unregister_sysfs(sbi);
> +free_compress_inode:
> + f2fs_destroy_compress_inode(sbi);
> free_root_inode:
> dput(sb->s_root);
> sb->s_root = NULL;
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index c6cc0a566ef5..2dcc63fe8494 100644
> --- a/include/linux/f2fs_fs.h
> +++ b/include/linux/f2fs_fs.h
> @@ -34,6 +34,7 @@
> #define F2FS_ROOT_INO(sbi) ((sbi)->root_ino_num)
> #define F2FS_NODE_INO(sbi) ((sbi)->node_ino_num)
> #define F2FS_META_INO(sbi) ((sbi)->meta_ino_num)
> +#define F2FS_COMPRESS_INO(sbi) (NM_I(sbi)->max_nid)
>
> #define F2FS_MAX_QUOTAS 3
>
> --
> 2.29.2

2021-01-11 10:35:13

by Chao Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/11 17:48, Jaegeuk Kim wrote:
> Hi Chao,
>
> After quick test of fsstress w/ fault injection, it gave wrong block address
> errors. Could you please run the test a bit?

Jaegeuk,

Oh, I've covered with fstest cases and there is no such error message, let me
try fault injection + SPO case soon.

Thanks,

2021-01-11 11:48:38

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/11 18:31, Chao Yu wrote:
> On 2021/1/11 17:48, Jaegeuk Kim wrote:
>> Hi Chao,
>>
>> After quick test of fsstress w/ fault injection, it gave wrong block address
>> errors. Could you please run the test a bit?
>
> Jaegeuk,
>
> Oh, I've covered with fstest cases and there is no such error message, let me
> try fault injection + SPO case soon.

Till now, I haven't see any problem... will let the test run for longer time in
this night.

Could you share me detailed error message you encounter?

Thanks,

>
> Thanks,
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
>

2021-01-12 10:54:46

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/11 19:45, Chao Yu wrote:
> On 2021/1/11 18:31, Chao Yu wrote:
>> On 2021/1/11 17:48, Jaegeuk Kim wrote:
>>> Hi Chao,
>>>
>>> After quick test of fsstress w/ fault injection, it gave wrong block address
>>> errors. Could you please run the test a bit?
>>
>> Jaegeuk,
>>
>> Oh, I've covered with fstest cases and there is no such error message, let me
>> try fault injection + SPO case soon.
>
> Till now, I haven't see any problem... will let the test run for longer time in
> this night.
>
> Could you share me detailed error message you encounter?

Still, I don't see wrong block address error...

Did the error occur from below path:

- f2fs_end_read_compressed_page
- f2fs_cache_compressed_page
- f2fs_is_valid_blkaddr

>
> Thanks,
>
>>
>> Thanks,
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>> .
>>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
>

2021-01-12 11:10:32

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/12, Chao Yu wrote:
> On 2021/1/11 19:45, Chao Yu wrote:
> > On 2021/1/11 18:31, Chao Yu wrote:
> > > On 2021/1/11 17:48, Jaegeuk Kim wrote:
> > > > Hi Chao,
> > > >
> > > > After quick test of fsstress w/ fault injection, it gave wrong block address
> > > > errors. Could you please run the test a bit?
> > >
> > > Jaegeuk,
> > >
> > > Oh, I've covered with fstest cases and there is no such error message, let me
> > > try fault injection + SPO case soon.
> >
> > Till now, I haven't see any problem... will let the test run for longer time in
> > this night.
> >
> > Could you share me detailed error message you encounter?
>
> Still, I don't see wrong block address error...
>
> Did the error occur from below path:
>
> - f2fs_end_read_compressed_page
> - f2fs_cache_compressed_page
> - f2fs_is_valid_blkaddr

[58690.176668] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
[58690.180563] CPU: 0 PID: 29371 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
[58690.186466] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
[58690.189352] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
[58690.193366] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
[58690.212479] RSP: 0018:ffffb429022dfa60 EFLAGS: 00010206
[58690.218410] RAX: 0000000000000001 RBX: 00000000000078af RCX: 0000000000200000
[58690.222473] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
[58690.227349] RBP: ffffb429022dfb10 R08: 0000000000000000 R09: 0000000000000000
[58690.234425] R10: ffff9c3af1f78200 R11: 0000000000000001 R12: 0000000000000000
[58690.238503] R13: ffff9c3b84041000 R14: fffff5cc8166f5c0 R15: ffff9c3af1f78200
[58690.242455] FS: 00007f0fee9d4b80(0000) GS:ffff9c3bbbc00000(0000) knlGS:0000000000000000
[58690.246401] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[58690.250471] CR2: 0000563b839c1000 CR3: 000000002cb0e004 CR4: 0000000000370ef0
[58690.250471] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[58690.258758] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[58690.262464] Call Trace:
[58690.262464] prepare_compress_overwrite+0x380/0x510 [f2fs]
[58690.266489] ? xas_load+0x9/0x80
[58690.270452] f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
[58690.274466] f2fs_write_begin+0x81e/0x1120 [f2fs]
[58690.277213] generic_perform_write+0xc2/0x1c0
[58690.278698] __generic_file_write_iter+0x167/0x1d0
[58690.286472] f2fs_file_write_iter+0x39e/0x590 [f2fs]
[58690.290398] new_sync_write+0x117/0x1b0
[58690.290461] vfs_write+0x185/0x250
[58690.295197] ksys_write+0x67/0xe0
[58690.298173] __x64_sys_write+0x1a/0x20
[58690.298437] do_syscall_64+0x38/0x90
[58690.298437] entry_SYSCALL_64_after_hwframe+0x44/0xa9

[58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
[58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
[58691.077338] ------------[ cut here ]------------
[58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
[58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
[58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
[58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
[58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
[58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
[58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
[58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
[58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
[58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
[58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
[58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
[58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
[58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
[58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
[58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[58691.176163] Call Trace:
[58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
[58691.180549] ? newidle_balance+0x253/0x3d0
[58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
[58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
[58691.192489] process_one_work+0x221/0x3a0
[58691.194482] worker_thread+0x4d/0x3f0
[58691.198867] kthread+0x114/0x150
[58691.202243] ? process_one_work+0x3a0/0x3a0
[58691.205367] ? kthread_park+0x90/0x90
[58691.208244] ret_from_fork+0x22/0x30

[58691.910477] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of generic_perform_write+0xc2/0x1c0
[58692.161597] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#3] SMP PTI
[58692.166490] CPU: 7 PID: 29391 Comm: fsstress Tainted: G D W O 5.11.0-rc3-custom #1
[58692.170509] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
[58692.174440] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
[58692.179095] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
[58692.182436] RSP: 0018:ffffb4290236f710 EFLAGS: 00010206
[58692.182436] RAX: 0000000000000001 RBX: 00000000001053b4 RCX: 0000000000200000
[58692.190453] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
[58692.194397] RBP: ffffb4290236f7c0 R08: 0000000000000000 R09: 0000000000000000
[58692.194397] R10: ffff9c3af1f79d90 R11: 0000000000000001 R12: ffff9c3b8d90fac0
[58692.198431] R13: ffff9c3b84041000 R14: fffff5cc8286b040 R15: ffff9c3af1f79d90
[58692.202375] FS: 00007f629dff5b80(0000) GS:ffff9c3bbbdc0000(0000) knlGS:0000000000000000
[58692.202375] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[58692.206461] CR2: 00007f224daa9410 CR3: 000000007ece8002 CR4: 0000000000370ee0
[58692.210457] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[58692.214403] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[58692.214437] Call Trace:
[58692.218448] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
[58692.218448] f2fs_readahead+0x47/0x90 [f2fs]
[58692.222514] read_pages+0x8e/0x280
[58692.222514] ? add_to_page_cache_lru+0x78/0xd0
[58692.226487] page_cache_ra_unbounded+0x137/0x1f0
[58692.226487] do_page_cache_ra+0x3d/0x40
[58692.226487] ondemand_readahead+0x17d/0x2e0
[58692.230524] ? find_get_pages_contig+0x12a/0x250
[58692.234404] page_cache_sync_ra+0xd4/0xe0
[58692.234404] generic_file_buffered_read_get_pages+0x126/0x8d0
[58692.238425] ? arch_stack_walk+0x9e/0xf0
[58692.238425] generic_file_buffered_read+0x113/0x4a0
[58692.242366] ? __slab_free+0x25e/0x380
[58692.242366] ? f2fs_file_write_iter+0x9f/0x590 [f2fs]
[58692.246402] generic_file_read_iter+0xdf/0x140
[58692.246423] f2fs_file_read_iter+0x34/0xb0 [f2fs]
[58692.246423] generic_file_splice_read+0xf7/0x1a0
[58692.250393] do_splice_to+0x6e/0xa0
[58692.250393] splice_direct_to_actor+0xcd/0x250
[58692.254439] ? wait_for_space+0x90/0x90
[58692.254439] do_splice_direct+0x89/0xd0
[58692.258493] vfs_copy_file_range+0x1b1/0x470
[58692.258493] __x64_sys_copy_file_range+0xd4/0x200
[58692.261742] do_syscall_64+0x38/0x90
[58692.263167] entry_SYSCALL_64_after_hwframe+0x44/0xa9

>
> >
> > Thanks,
> >
> > >
> > > Thanks,
> > >
> > >
> > >
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > [email protected]
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > .
> > >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > .
> >

2021-01-12 12:11:42

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/12 10:04, Jaegeuk Kim wrote:
> On 01/12, Chao Yu wrote:
>> On 2021/1/11 19:45, Chao Yu wrote:
>>> On 2021/1/11 18:31, Chao Yu wrote:
>>>> On 2021/1/11 17:48, Jaegeuk Kim wrote:
>>>>> Hi Chao,
>>>>>
>>>>> After quick test of fsstress w/ fault injection, it gave wrong block address
>>>>> errors. Could you please run the test a bit?
>>>>
>>>> Jaegeuk,
>>>>
>>>> Oh, I've covered with fstest cases and there is no such error message, let me
>>>> try fault injection + SPO case soon.
>>>
>>> Till now, I haven't see any problem... will let the test run for longer time in
>>> this night.
>>>
>>> Could you share me detailed error message you encounter?
>>
>> Still, I don't see wrong block address error...
>>
>> Did the error occur from below path:
>>
>> - f2fs_end_read_compressed_page
>> - f2fs_cache_compressed_page
>> - f2fs_is_valid_blkaddr
>
> [58690.176668] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> [58690.180563] CPU: 0 PID: 29371 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> [58690.186466] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> [58690.189352] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> [58690.193366] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> [58690.212479] RSP: 0018:ffffb429022dfa60 EFLAGS: 00010206
> [58690.218410] RAX: 0000000000000001 RBX: 00000000000078af RCX: 0000000000200000
> [58690.222473] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> [58690.227349] RBP: ffffb429022dfb10 R08: 0000000000000000 R09: 0000000000000000
> [58690.234425] R10: ffff9c3af1f78200 R11: 0000000000000001 R12: 0000000000000000
> [58690.238503] R13: ffff9c3b84041000 R14: fffff5cc8166f5c0 R15: ffff9c3af1f78200
> [58690.242455] FS: 00007f0fee9d4b80(0000) GS:ffff9c3bbbc00000(0000) knlGS:0000000000000000
> [58690.246401] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [58690.250471] CR2: 0000563b839c1000 CR3: 000000002cb0e004 CR4: 0000000000370ef0
> [58690.250471] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [58690.258758] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [58690.262464] Call Trace:
> [58690.262464] prepare_compress_overwrite+0x380/0x510 [f2fs]
> [58690.266489] ? xas_load+0x9/0x80
> [58690.270452] f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
> [58690.274466] f2fs_write_begin+0x81e/0x1120 [f2fs]
> [58690.277213] generic_perform_write+0xc2/0x1c0
> [58690.278698] __generic_file_write_iter+0x167/0x1d0
> [58690.286472] f2fs_file_write_iter+0x39e/0x590 [f2fs]
> [58690.290398] new_sync_write+0x117/0x1b0
> [58690.290461] vfs_write+0x185/0x250
> [58690.295197] ksys_write+0x67/0xe0
> [58690.298173] __x64_sys_write+0x1a/0x20
> [58690.298437] do_syscall_64+0x38/0x90
> [58690.298437] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> [58691.077338] ------------[ cut here ]------------
> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [58691.176163] Call Trace:
> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> [58691.180549] ? newidle_balance+0x253/0x3d0
> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> [58691.192489] process_one_work+0x221/0x3a0
> [58691.194482] worker_thread+0x4d/0x3f0
> [58691.198867] kthread+0x114/0x150
> [58691.202243] ? process_one_work+0x3a0/0x3a0
> [58691.205367] ? kthread_park+0x90/0x90
> [58691.208244] ret_from_fork+0x22/0x30
>
> [58691.910477] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of generic_perform_write+0xc2/0x1c0
> [58692.161597] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#3] SMP PTI
> [58692.166490] CPU: 7 PID: 29391 Comm: fsstress Tainted: G D W O 5.11.0-rc3-custom #1
> [58692.170509] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> [58692.174440] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> [58692.179095] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> [58692.182436] RSP: 0018:ffffb4290236f710 EFLAGS: 00010206
> [58692.182436] RAX: 0000000000000001 RBX: 00000000001053b4 RCX: 0000000000200000
> [58692.190453] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> [58692.194397] RBP: ffffb4290236f7c0 R08: 0000000000000000 R09: 0000000000000000
> [58692.194397] R10: ffff9c3af1f79d90 R11: 0000000000000001 R12: ffff9c3b8d90fac0
> [58692.198431] R13: ffff9c3b84041000 R14: fffff5cc8286b040 R15: ffff9c3af1f79d90
> [58692.202375] FS: 00007f629dff5b80(0000) GS:ffff9c3bbbdc0000(0000) knlGS:0000000000000000
> [58692.202375] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [58692.206461] CR2: 00007f224daa9410 CR3: 000000007ece8002 CR4: 0000000000370ee0
> [58692.210457] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [58692.214403] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [58692.214437] Call Trace:
> [58692.218448] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> [58692.218448] f2fs_readahead+0x47/0x90 [f2fs]
> [58692.222514] read_pages+0x8e/0x280
> [58692.222514] ? add_to_page_cache_lru+0x78/0xd0
> [58692.226487] page_cache_ra_unbounded+0x137/0x1f0
> [58692.226487] do_page_cache_ra+0x3d/0x40
> [58692.226487] ondemand_readahead+0x17d/0x2e0
> [58692.230524] ? find_get_pages_contig+0x12a/0x250
> [58692.234404] page_cache_sync_ra+0xd4/0xe0
> [58692.234404] generic_file_buffered_read_get_pages+0x126/0x8d0
> [58692.238425] ? arch_stack_walk+0x9e/0xf0
> [58692.238425] generic_file_buffered_read+0x113/0x4a0
> [58692.242366] ? __slab_free+0x25e/0x380
> [58692.242366] ? f2fs_file_write_iter+0x9f/0x590 [f2fs]
> [58692.246402] generic_file_read_iter+0xdf/0x140
> [58692.246423] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> [58692.246423] generic_file_splice_read+0xf7/0x1a0
> [58692.250393] do_splice_to+0x6e/0xa0
> [58692.250393] splice_direct_to_actor+0xcd/0x250
> [58692.254439] ? wait_for_space+0x90/0x90
> [58692.254439] do_splice_direct+0x89/0xd0
> [58692.258493] vfs_copy_file_range+0x1b1/0x470
> [58692.258493] __x64_sys_copy_file_range+0xd4/0x200
> [58692.261742] do_syscall_64+0x38/0x90
> [58692.263167] entry_SYSCALL_64_after_hwframe+0x44/0xa9

Jaegeuk,

I rerun por_fsstress testcase on a mountpoint which enables compression,
sysfile quota, compress_cache, but it doesn't reproduce this issue.

Not sure, below diff would help to solve this issue, could you please
help to try this?

From ac15dffa459ee734f3d66ea6bb7d936c8d7e0565 Mon Sep 17 00:00:00 2001
From: Chao Yu <[email protected]>
Date: Tue, 12 Jan 2021 15:26:59 +0800
Subject: [PATCH] f2fs: fix wrong blkaddr

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/data.c | 8 ++++++++
fs/f2fs/super.c | 7 +++++++
2 files changed, 15 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f8578fad7cd1..b3973494b102 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3679,6 +3679,9 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,

clear_cold_data(page);

+ if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
+ f2fs_invalidate_compress_pages(sbi, inode->i_ino);
+
if (IS_ATOMIC_WRITTEN_PAGE(page))
return f2fs_drop_inmem_page(inode, page);

@@ -3695,6 +3698,11 @@ int f2fs_release_page(struct page *page, gfp_t wait)
if (IS_ATOMIC_WRITTEN_PAGE(page))
return 0;

+ if (test_opt(F2FS_P_SB(page), COMPRESS_CACHE) &&
+ f2fs_compressed_file(page->mapping->host))
+ f2fs_invalidate_compress_pages(F2FS_P_SB(page),
+ page->mapping->host->i_ino);
+
clear_cold_data(page);
f2fs_clear_page_private(page);
return 1;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8b5975731439..a44dfc9a594a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1915,6 +1915,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT);
bool no_io_align = !F2FS_IO_ALIGNED(sbi);
bool no_atgc = !test_opt(sbi, ATGC);
+ bool no_compress_cache = !test_opt(sbi, COMPRESS_CACHE);
bool checkpoint_changed;
#ifdef CONFIG_QUOTA
int i, j;
@@ -2007,6 +2008,12 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
goto restore_opts;
}

+ if (no_compress_cache == !!test_opt(sbi, COMPRESS_CACHE)) {
+ err = -EINVAL;
+ f2fs_warn(sbi, "switch compress_cache option is not allowed");
+ goto restore_opts;
+ }
+
if ((*flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
err = -EINVAL;
f2fs_warn(sbi, "disabling checkpoint not compatible with read-only");
--
2.29.2



>
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>> .
>>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>> .
>>>
> .
>

2021-01-13 03:19:44

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/12, Chao Yu wrote:
> On 2021/1/12 10:04, Jaegeuk Kim wrote:
> > On 01/12, Chao Yu wrote:
> > > On 2021/1/11 19:45, Chao Yu wrote:
> > > > On 2021/1/11 18:31, Chao Yu wrote:
> > > > > On 2021/1/11 17:48, Jaegeuk Kim wrote:
> > > > > > Hi Chao,
> > > > > >
> > > > > > After quick test of fsstress w/ fault injection, it gave wrong block address
> > > > > > errors. Could you please run the test a bit?
> > > > >
> > > > > Jaegeuk,
> > > > >
> > > > > Oh, I've covered with fstest cases and there is no such error message, let me
> > > > > try fault injection + SPO case soon.
> > > >
> > > > Till now, I haven't see any problem... will let the test run for longer time in
> > > > this night.
> > > >
> > > > Could you share me detailed error message you encounter?
> > >
> > > Still, I don't see wrong block address error...
> > >
> > > Did the error occur from below path:
> > >
> > > - f2fs_end_read_compressed_page
> > > - f2fs_cache_compressed_page
> > > - f2fs_is_valid_blkaddr
> >
> > [58690.176668] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > [58690.180563] CPU: 0 PID: 29371 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > [58690.186466] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > [58690.189352] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> > [58690.193366] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> > [58690.212479] RSP: 0018:ffffb429022dfa60 EFLAGS: 00010206
> > [58690.218410] RAX: 0000000000000001 RBX: 00000000000078af RCX: 0000000000200000
> > [58690.222473] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> > [58690.227349] RBP: ffffb429022dfb10 R08: 0000000000000000 R09: 0000000000000000
> > [58690.234425] R10: ffff9c3af1f78200 R11: 0000000000000001 R12: 0000000000000000
> > [58690.238503] R13: ffff9c3b84041000 R14: fffff5cc8166f5c0 R15: ffff9c3af1f78200
> > [58690.242455] FS: 00007f0fee9d4b80(0000) GS:ffff9c3bbbc00000(0000) knlGS:0000000000000000
> > [58690.246401] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [58690.250471] CR2: 0000563b839c1000 CR3: 000000002cb0e004 CR4: 0000000000370ef0
> > [58690.250471] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [58690.258758] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [58690.262464] Call Trace:
> > [58690.262464] prepare_compress_overwrite+0x380/0x510 [f2fs]
> > [58690.266489] ? xas_load+0x9/0x80
> > [58690.270452] f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
> > [58690.274466] f2fs_write_begin+0x81e/0x1120 [f2fs]
> > [58690.277213] generic_perform_write+0xc2/0x1c0
> > [58690.278698] __generic_file_write_iter+0x167/0x1d0
> > [58690.286472] f2fs_file_write_iter+0x39e/0x590 [f2fs]
> > [58690.290398] new_sync_write+0x117/0x1b0
> > [58690.290461] vfs_write+0x185/0x250
> > [58690.295197] ksys_write+0x67/0xe0
> > [58690.298173] __x64_sys_write+0x1a/0x20
> > [58690.298437] do_syscall_64+0x38/0x90
> > [58690.298437] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > [58691.077338] ------------[ cut here ]------------
> > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [58691.176163] Call Trace:
> > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > [58691.180549] ? newidle_balance+0x253/0x3d0
> > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > [58691.192489] process_one_work+0x221/0x3a0
> > [58691.194482] worker_thread+0x4d/0x3f0
> > [58691.198867] kthread+0x114/0x150
> > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > [58691.205367] ? kthread_park+0x90/0x90
> > [58691.208244] ret_from_fork+0x22/0x30
> >
> > [58691.910477] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of generic_perform_write+0xc2/0x1c0
> > [58692.161597] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#3] SMP PTI
> > [58692.166490] CPU: 7 PID: 29391 Comm: fsstress Tainted: G D W O 5.11.0-rc3-custom #1
> > [58692.170509] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > [58692.174440] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> > [58692.179095] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> > [58692.182436] RSP: 0018:ffffb4290236f710 EFLAGS: 00010206
> > [58692.182436] RAX: 0000000000000001 RBX: 00000000001053b4 RCX: 0000000000200000
> > [58692.190453] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> > [58692.194397] RBP: ffffb4290236f7c0 R08: 0000000000000000 R09: 0000000000000000
> > [58692.194397] R10: ffff9c3af1f79d90 R11: 0000000000000001 R12: ffff9c3b8d90fac0
> > [58692.198431] R13: ffff9c3b84041000 R14: fffff5cc8286b040 R15: ffff9c3af1f79d90
> > [58692.202375] FS: 00007f629dff5b80(0000) GS:ffff9c3bbbdc0000(0000) knlGS:0000000000000000
> > [58692.202375] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [58692.206461] CR2: 00007f224daa9410 CR3: 000000007ece8002 CR4: 0000000000370ee0
> > [58692.210457] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [58692.214403] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [58692.214437] Call Trace:
> > [58692.218448] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > [58692.218448] f2fs_readahead+0x47/0x90 [f2fs]
> > [58692.222514] read_pages+0x8e/0x280
> > [58692.222514] ? add_to_page_cache_lru+0x78/0xd0
> > [58692.226487] page_cache_ra_unbounded+0x137/0x1f0
> > [58692.226487] do_page_cache_ra+0x3d/0x40
> > [58692.226487] ondemand_readahead+0x17d/0x2e0
> > [58692.230524] ? find_get_pages_contig+0x12a/0x250
> > [58692.234404] page_cache_sync_ra+0xd4/0xe0
> > [58692.234404] generic_file_buffered_read_get_pages+0x126/0x8d0
> > [58692.238425] ? arch_stack_walk+0x9e/0xf0
> > [58692.238425] generic_file_buffered_read+0x113/0x4a0
> > [58692.242366] ? __slab_free+0x25e/0x380
> > [58692.242366] ? f2fs_file_write_iter+0x9f/0x590 [f2fs]
> > [58692.246402] generic_file_read_iter+0xdf/0x140
> > [58692.246423] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > [58692.246423] generic_file_splice_read+0xf7/0x1a0
> > [58692.250393] do_splice_to+0x6e/0xa0
> > [58692.250393] splice_direct_to_actor+0xcd/0x250
> > [58692.254439] ? wait_for_space+0x90/0x90
> > [58692.254439] do_splice_direct+0x89/0xd0
> > [58692.258493] vfs_copy_file_range+0x1b1/0x470
> > [58692.258493] __x64_sys_copy_file_range+0xd4/0x200
> > [58692.261742] do_syscall_64+0x38/0x90
> > [58692.263167] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Jaegeuk,
>
> I rerun por_fsstress testcase on a mountpoint which enables compression,
> sysfile quota, compress_cache, but it doesn't reproduce this issue.

Have you enable compress_extension=*?

>
> Not sure, below diff would help to solve this issue, could you please
> help to try this?

No luck with this diff.

>
> From ac15dffa459ee734f3d66ea6bb7d936c8d7e0565 Mon Sep 17 00:00:00 2001
> From: Chao Yu <[email protected]>
> Date: Tue, 12 Jan 2021 15:26:59 +0800
> Subject: [PATCH] f2fs: fix wrong blkaddr
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/data.c | 8 ++++++++
> fs/f2fs/super.c | 7 +++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index f8578fad7cd1..b3973494b102 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3679,6 +3679,9 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>
> clear_cold_data(page);
>
> + if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
> + f2fs_invalidate_compress_pages(sbi, inode->i_ino);
> +
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return f2fs_drop_inmem_page(inode, page);
>
> @@ -3695,6 +3698,11 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return 0;
>
> + if (test_opt(F2FS_P_SB(page), COMPRESS_CACHE) &&
> + f2fs_compressed_file(page->mapping->host))
> + f2fs_invalidate_compress_pages(F2FS_P_SB(page),
> + page->mapping->host->i_ino);
> +
> clear_cold_data(page);
> f2fs_clear_page_private(page);
> return 1;
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 8b5975731439..a44dfc9a594a 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1915,6 +1915,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT);
> bool no_io_align = !F2FS_IO_ALIGNED(sbi);
> bool no_atgc = !test_opt(sbi, ATGC);
> + bool no_compress_cache = !test_opt(sbi, COMPRESS_CACHE);
> bool checkpoint_changed;
> #ifdef CONFIG_QUOTA
> int i, j;
> @@ -2007,6 +2008,12 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> goto restore_opts;
> }
>
> + if (no_compress_cache == !!test_opt(sbi, COMPRESS_CACHE)) {
> + err = -EINVAL;
> + f2fs_warn(sbi, "switch compress_cache option is not allowed");
> + goto restore_opts;
> + }
> +
> if ((*flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
> err = -EINVAL;
> f2fs_warn(sbi, "disabling checkpoint not compatible with read-only");
> --
> 2.29.2
>
>
>
> >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > Linux-f2fs-devel mailing list
> > > > > [email protected]
> > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > > .
> > > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > Linux-f2fs-devel mailing list
> > > > [email protected]
> > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > .
> > > >
> > .
> >

2021-01-13 03:36:06

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/13 6:36, Jaegeuk Kim wrote:
> On 01/12, Chao Yu wrote:
>> On 2021/1/12 10:04, Jaegeuk Kim wrote:
>>> On 01/12, Chao Yu wrote:
>>>> On 2021/1/11 19:45, Chao Yu wrote:
>>>>> On 2021/1/11 18:31, Chao Yu wrote:
>>>>>> On 2021/1/11 17:48, Jaegeuk Kim wrote:
>>>>>>> Hi Chao,
>>>>>>>
>>>>>>> After quick test of fsstress w/ fault injection, it gave wrong block address
>>>>>>> errors. Could you please run the test a bit?
>>>>>>
>>>>>> Jaegeuk,
>>>>>>
>>>>>> Oh, I've covered with fstest cases and there is no such error message, let me
>>>>>> try fault injection + SPO case soon.
>>>>>
>>>>> Till now, I haven't see any problem... will let the test run for longer time in
>>>>> this night.
>>>>>
>>>>> Could you share me detailed error message you encounter?
>>>>
>>>> Still, I don't see wrong block address error...
>>>>
>>>> Did the error occur from below path:
>>>>
>>>> - f2fs_end_read_compressed_page
>>>> - f2fs_cache_compressed_page
>>>> - f2fs_is_valid_blkaddr
>>>
>>> [58690.176668] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
>>> [58690.180563] CPU: 0 PID: 29371 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
>>> [58690.186466] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>> [58690.189352] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
>>> [58690.193366] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
>>> [58690.212479] RSP: 0018:ffffb429022dfa60 EFLAGS: 00010206
>>> [58690.218410] RAX: 0000000000000001 RBX: 00000000000078af RCX: 0000000000200000
>>> [58690.222473] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
>>> [58690.227349] RBP: ffffb429022dfb10 R08: 0000000000000000 R09: 0000000000000000
>>> [58690.234425] R10: ffff9c3af1f78200 R11: 0000000000000001 R12: 0000000000000000
>>> [58690.238503] R13: ffff9c3b84041000 R14: fffff5cc8166f5c0 R15: ffff9c3af1f78200
>>> [58690.242455] FS: 00007f0fee9d4b80(0000) GS:ffff9c3bbbc00000(0000) knlGS:0000000000000000
>>> [58690.246401] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [58690.250471] CR2: 0000563b839c1000 CR3: 000000002cb0e004 CR4: 0000000000370ef0
>>> [58690.250471] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [58690.258758] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [58690.262464] Call Trace:
>>> [58690.262464] prepare_compress_overwrite+0x380/0x510 [f2fs]
>>> [58690.266489] ? xas_load+0x9/0x80
>>> [58690.270452] f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
>>> [58690.274466] f2fs_write_begin+0x81e/0x1120 [f2fs]
>>> [58690.277213] generic_perform_write+0xc2/0x1c0
>>> [58690.278698] __generic_file_write_iter+0x167/0x1d0
>>> [58690.286472] f2fs_file_write_iter+0x39e/0x590 [f2fs]
>>> [58690.290398] new_sync_write+0x117/0x1b0
>>> [58690.290461] vfs_write+0x185/0x250
>>> [58690.295197] ksys_write+0x67/0xe0
>>> [58690.298173] __x64_sys_write+0x1a/0x20
>>> [58690.298437] do_syscall_64+0x38/0x90
>>> [58690.298437] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>
>>> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
>>> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
>>> [58691.077338] ------------[ cut here ]------------
>>> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
>>> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
>>> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
>>> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
>>> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
>>> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
>>> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
>>> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
>>> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
>>> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
>>> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
>>> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
>>> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [58691.176163] Call Trace:
>>> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
>>> [58691.180549] ? newidle_balance+0x253/0x3d0
>>> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
>>> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
>>> [58691.192489] process_one_work+0x221/0x3a0
>>> [58691.194482] worker_thread+0x4d/0x3f0
>>> [58691.198867] kthread+0x114/0x150
>>> [58691.202243] ? process_one_work+0x3a0/0x3a0
>>> [58691.205367] ? kthread_park+0x90/0x90
>>> [58691.208244] ret_from_fork+0x22/0x30
>>>
>>> [58691.910477] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of generic_perform_write+0xc2/0x1c0
>>> [58692.161597] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#3] SMP PTI
>>> [58692.166490] CPU: 7 PID: 29391 Comm: fsstress Tainted: G D W O 5.11.0-rc3-custom #1
>>> [58692.170509] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>> [58692.174440] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
>>> [58692.179095] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
>>> [58692.182436] RSP: 0018:ffffb4290236f710 EFLAGS: 00010206
>>> [58692.182436] RAX: 0000000000000001 RBX: 00000000001053b4 RCX: 0000000000200000
>>> [58692.190453] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
>>> [58692.194397] RBP: ffffb4290236f7c0 R08: 0000000000000000 R09: 0000000000000000
>>> [58692.194397] R10: ffff9c3af1f79d90 R11: 0000000000000001 R12: ffff9c3b8d90fac0
>>> [58692.198431] R13: ffff9c3b84041000 R14: fffff5cc8286b040 R15: ffff9c3af1f79d90
>>> [58692.202375] FS: 00007f629dff5b80(0000) GS:ffff9c3bbbdc0000(0000) knlGS:0000000000000000
>>> [58692.202375] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [58692.206461] CR2: 00007f224daa9410 CR3: 000000007ece8002 CR4: 0000000000370ee0
>>> [58692.210457] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [58692.214403] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [58692.214437] Call Trace:
>>> [58692.218448] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
>>> [58692.218448] f2fs_readahead+0x47/0x90 [f2fs]
>>> [58692.222514] read_pages+0x8e/0x280
>>> [58692.222514] ? add_to_page_cache_lru+0x78/0xd0
>>> [58692.226487] page_cache_ra_unbounded+0x137/0x1f0
>>> [58692.226487] do_page_cache_ra+0x3d/0x40
>>> [58692.226487] ondemand_readahead+0x17d/0x2e0
>>> [58692.230524] ? find_get_pages_contig+0x12a/0x250
>>> [58692.234404] page_cache_sync_ra+0xd4/0xe0
>>> [58692.234404] generic_file_buffered_read_get_pages+0x126/0x8d0
>>> [58692.238425] ? arch_stack_walk+0x9e/0xf0
>>> [58692.238425] generic_file_buffered_read+0x113/0x4a0
>>> [58692.242366] ? __slab_free+0x25e/0x380
>>> [58692.242366] ? f2fs_file_write_iter+0x9f/0x590 [f2fs]
>>> [58692.246402] generic_file_read_iter+0xdf/0x140
>>> [58692.246423] f2fs_file_read_iter+0x34/0xb0 [f2fs]
>>> [58692.246423] generic_file_splice_read+0xf7/0x1a0
>>> [58692.250393] do_splice_to+0x6e/0xa0
>>> [58692.250393] splice_direct_to_actor+0xcd/0x250
>>> [58692.254439] ? wait_for_space+0x90/0x90
>>> [58692.254439] do_splice_direct+0x89/0xd0
>>> [58692.258493] vfs_copy_file_range+0x1b1/0x470
>>> [58692.258493] __x64_sys_copy_file_range+0xd4/0x200
>>> [58692.261742] do_syscall_64+0x38/0x90
>>> [58692.263167] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Jaegeuk,
>>
>> I rerun por_fsstress testcase on a mountpoint which enables compression,
>> sysfile quota, compress_cache, but it doesn't reproduce this issue.
>
> Have you enable compress_extension=*?

Yup, I forced to enable this in default_options(), and during the test, I
have confirmed that regular inode is compressed by using lsattr.

>
>>
>> Not sure, below diff would help to solve this issue, could you please
>> help to try this?
>
> No luck with this diff.

If we disable compress_cache option, there is no issue during por_fsstress test,
right?

Thanks,

>
>>
>> From ac15dffa459ee734f3d66ea6bb7d936c8d7e0565 Mon Sep 17 00:00:00 2001
>> From: Chao Yu <[email protected]>
>> Date: Tue, 12 Jan 2021 15:26:59 +0800
>> Subject: [PATCH] f2fs: fix wrong blkaddr
>>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> fs/f2fs/data.c | 8 ++++++++
>> fs/f2fs/super.c | 7 +++++++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index f8578fad7cd1..b3973494b102 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -3679,6 +3679,9 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>>
>> clear_cold_data(page);
>>
>> + if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
>> + f2fs_invalidate_compress_pages(sbi, inode->i_ino);
>> +
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return f2fs_drop_inmem_page(inode, page);
>>
>> @@ -3695,6 +3698,11 @@ int f2fs_release_page(struct page *page, gfp_t wait)
>> if (IS_ATOMIC_WRITTEN_PAGE(page))
>> return 0;
>>
>> + if (test_opt(F2FS_P_SB(page), COMPRESS_CACHE) &&
>> + f2fs_compressed_file(page->mapping->host))
>> + f2fs_invalidate_compress_pages(F2FS_P_SB(page),
>> + page->mapping->host->i_ino);
>> +
>> clear_cold_data(page);
>> f2fs_clear_page_private(page);
>> return 1;
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 8b5975731439..a44dfc9a594a 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1915,6 +1915,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>> bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT);
>> bool no_io_align = !F2FS_IO_ALIGNED(sbi);
>> bool no_atgc = !test_opt(sbi, ATGC);
>> + bool no_compress_cache = !test_opt(sbi, COMPRESS_CACHE);
>> bool checkpoint_changed;
>> #ifdef CONFIG_QUOTA
>> int i, j;
>> @@ -2007,6 +2008,12 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>> goto restore_opts;
>> }
>>
>> + if (no_compress_cache == !!test_opt(sbi, COMPRESS_CACHE)) {
>> + err = -EINVAL;
>> + f2fs_warn(sbi, "switch compress_cache option is not allowed");
>> + goto restore_opts;
>> + }
>> +
>> if ((*flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
>> err = -EINVAL;
>> f2fs_warn(sbi, "disabling checkpoint not compatible with read-only");
>> --
>> 2.29.2
>>
>>
>>
>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-f2fs-devel mailing list
>>>>>> [email protected]
>>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-f2fs-devel mailing list
>>>>> [email protected]
>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>> .
>>>>>
>>> .
>>>
> .
>

2021-01-13 15:44:45

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/13, Chao Yu wrote:
> On 2021/1/13 6:36, Jaegeuk Kim wrote:
> > On 01/12, Chao Yu wrote:
> > > On 2021/1/12 10:04, Jaegeuk Kim wrote:
> > > > On 01/12, Chao Yu wrote:
> > > > > On 2021/1/11 19:45, Chao Yu wrote:
> > > > > > On 2021/1/11 18:31, Chao Yu wrote:
> > > > > > > On 2021/1/11 17:48, Jaegeuk Kim wrote:
> > > > > > > > Hi Chao,
> > > > > > > >
> > > > > > > > After quick test of fsstress w/ fault injection, it gave wrong block address
> > > > > > > > errors. Could you please run the test a bit?
> > > > > > >
> > > > > > > Jaegeuk,
> > > > > > >
> > > > > > > Oh, I've covered with fstest cases and there is no such error message, let me
> > > > > > > try fault injection + SPO case soon.
> > > > > >
> > > > > > Till now, I haven't see any problem... will let the test run for longer time in
> > > > > > this night.
> > > > > >
> > > > > > Could you share me detailed error message you encounter?
> > > > >
> > > > > Still, I don't see wrong block address error...
> > > > >
> > > > > Did the error occur from below path:
> > > > >
> > > > > - f2fs_end_read_compressed_page
> > > > > - f2fs_cache_compressed_page
> > > > > - f2fs_is_valid_blkaddr
> > > >
> > > > [58690.176668] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > > > [58690.180563] CPU: 0 PID: 29371 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > > > [58690.186466] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [58690.189352] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> > > > [58690.193366] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> > > > [58690.212479] RSP: 0018:ffffb429022dfa60 EFLAGS: 00010206
> > > > [58690.218410] RAX: 0000000000000001 RBX: 00000000000078af RCX: 0000000000200000
> > > > [58690.222473] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> > > > [58690.227349] RBP: ffffb429022dfb10 R08: 0000000000000000 R09: 0000000000000000
> > > > [58690.234425] R10: ffff9c3af1f78200 R11: 0000000000000001 R12: 0000000000000000
> > > > [58690.238503] R13: ffff9c3b84041000 R14: fffff5cc8166f5c0 R15: ffff9c3af1f78200
> > > > [58690.242455] FS: 00007f0fee9d4b80(0000) GS:ffff9c3bbbc00000(0000) knlGS:0000000000000000
> > > > [58690.246401] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [58690.250471] CR2: 0000563b839c1000 CR3: 000000002cb0e004 CR4: 0000000000370ef0
> > > > [58690.250471] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [58690.258758] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [58690.262464] Call Trace:
> > > > [58690.262464] prepare_compress_overwrite+0x380/0x510 [f2fs]
> > > > [58690.266489] ? xas_load+0x9/0x80
> > > > [58690.270452] f2fs_prepare_compress_overwrite+0x5f/0x80 [f2fs]
> > > > [58690.274466] f2fs_write_begin+0x81e/0x1120 [f2fs]
> > > > [58690.277213] generic_perform_write+0xc2/0x1c0
> > > > [58690.278698] __generic_file_write_iter+0x167/0x1d0
> > > > [58690.286472] f2fs_file_write_iter+0x39e/0x590 [f2fs]
> > > > [58690.290398] new_sync_write+0x117/0x1b0
> > > > [58690.290461] vfs_write+0x185/0x250
> > > > [58690.295197] ksys_write+0x67/0xe0
> > > > [58690.298173] __x64_sys_write+0x1a/0x20
> > > > [58690.298437] do_syscall_64+0x38/0x90
> > > > [58690.298437] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > >
> > > > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > > > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > > > [58691.077338] ------------[ cut here ]------------
> > > > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > > > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > > > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > > > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > > > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > > > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > > > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > > > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > > > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > > > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > > > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > > > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > > > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [58691.176163] Call Trace:
> > > > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > > > [58691.180549] ? newidle_balance+0x253/0x3d0
> > > > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > > > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > > > [58691.192489] process_one_work+0x221/0x3a0
> > > > [58691.194482] worker_thread+0x4d/0x3f0
> > > > [58691.198867] kthread+0x114/0x150
> > > > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > > > [58691.205367] ? kthread_park+0x90/0x90
> > > > [58691.208244] ret_from_fork+0x22/0x30
> > > >
> > > > [58691.910477] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of generic_perform_write+0xc2/0x1c0
> > > > [58692.161597] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#3] SMP PTI
> > > > [58692.166490] CPU: 7 PID: 29391 Comm: fsstress Tainted: G D W O 5.11.0-rc3-custom #1
> > > > [58692.170509] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [58692.174440] RIP: 0010:f2fs_read_multi_pages+0x413/0xa70 [f2fs]
> > > > [58692.179095] Code: ad 54 ff ff ff 4c 8b ad 68 ff ff ff 25 00 00 08 00 89 85 78 ff ff ff 49 8d 47 6c 48 89 85 70 ff ff ff 48 63 45 a0 49 8b 57 30 <4c> 8b 34 c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 98 56 40 c8 48
> > > > [58692.182436] RSP: 0018:ffffb4290236f710 EFLAGS: 00010206
> > > > [58692.182436] RAX: 0000000000000001 RBX: 00000000001053b4 RCX: 0000000000200000
> > > > [58692.190453] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc0a6872f RDI: 0000000000000246
> > > > [58692.194397] RBP: ffffb4290236f7c0 R08: 0000000000000000 R09: 0000000000000000
> > > > [58692.194397] R10: ffff9c3af1f79d90 R11: 0000000000000001 R12: ffff9c3b8d90fac0
> > > > [58692.198431] R13: ffff9c3b84041000 R14: fffff5cc8286b040 R15: ffff9c3af1f79d90
> > > > [58692.202375] FS: 00007f629dff5b80(0000) GS:ffff9c3bbbdc0000(0000) knlGS:0000000000000000
> > > > [58692.202375] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [58692.206461] CR2: 00007f224daa9410 CR3: 000000007ece8002 CR4: 0000000000370ee0
> > > > [58692.210457] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [58692.214403] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [58692.214437] Call Trace:
> > > > [58692.218448] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > > > [58692.218448] f2fs_readahead+0x47/0x90 [f2fs]
> > > > [58692.222514] read_pages+0x8e/0x280
> > > > [58692.222514] ? add_to_page_cache_lru+0x78/0xd0
> > > > [58692.226487] page_cache_ra_unbounded+0x137/0x1f0
> > > > [58692.226487] do_page_cache_ra+0x3d/0x40
> > > > [58692.226487] ondemand_readahead+0x17d/0x2e0
> > > > [58692.230524] ? find_get_pages_contig+0x12a/0x250
> > > > [58692.234404] page_cache_sync_ra+0xd4/0xe0
> > > > [58692.234404] generic_file_buffered_read_get_pages+0x126/0x8d0
> > > > [58692.238425] ? arch_stack_walk+0x9e/0xf0
> > > > [58692.238425] generic_file_buffered_read+0x113/0x4a0
> > > > [58692.242366] ? __slab_free+0x25e/0x380
> > > > [58692.242366] ? f2fs_file_write_iter+0x9f/0x590 [f2fs]
> > > > [58692.246402] generic_file_read_iter+0xdf/0x140
> > > > [58692.246423] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > > > [58692.246423] generic_file_splice_read+0xf7/0x1a0
> > > > [58692.250393] do_splice_to+0x6e/0xa0
> > > > [58692.250393] splice_direct_to_actor+0xcd/0x250
> > > > [58692.254439] ? wait_for_space+0x90/0x90
> > > > [58692.254439] do_splice_direct+0x89/0xd0
> > > > [58692.258493] vfs_copy_file_range+0x1b1/0x470
> > > > [58692.258493] __x64_sys_copy_file_range+0xd4/0x200
> > > > [58692.261742] do_syscall_64+0x38/0x90
> > > > [58692.263167] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > >
> > > Jaegeuk,
> > >
> > > I rerun por_fsstress testcase on a mountpoint which enables compression,
> > > sysfile quota, compress_cache, but it doesn't reproduce this issue.
> >
> > Have you enable compress_extension=*?
>
> Yup, I forced to enable this in default_options(), and during the test, I
> have confirmed that regular inode is compressed by using lsattr.
>
> >
> > >
> > > Not sure, below diff would help to solve this issue, could you please
> > > help to try this?
> >
> > No luck with this diff.
>
> If we disable compress_cache option, there is no issue during por_fsstress test,
> right?

Yes.

>
> Thanks,
>
> >
> > >
> > > From ac15dffa459ee734f3d66ea6bb7d936c8d7e0565 Mon Sep 17 00:00:00 2001
> > > From: Chao Yu <[email protected]>
> > > Date: Tue, 12 Jan 2021 15:26:59 +0800
> > > Subject: [PATCH] f2fs: fix wrong blkaddr
> > >
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > > fs/f2fs/data.c | 8 ++++++++
> > > fs/f2fs/super.c | 7 +++++++
> > > 2 files changed, 15 insertions(+)
> > >
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index f8578fad7cd1..b3973494b102 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -3679,6 +3679,9 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
> > >
> > > clear_cold_data(page);
> > >
> > > + if (test_opt(sbi, COMPRESS_CACHE) && f2fs_compressed_file(inode))
> > > + f2fs_invalidate_compress_pages(sbi, inode->i_ino);
> > > +
> > > if (IS_ATOMIC_WRITTEN_PAGE(page))
> > > return f2fs_drop_inmem_page(inode, page);
> > >
> > > @@ -3695,6 +3698,11 @@ int f2fs_release_page(struct page *page, gfp_t wait)
> > > if (IS_ATOMIC_WRITTEN_PAGE(page))
> > > return 0;
> > >
> > > + if (test_opt(F2FS_P_SB(page), COMPRESS_CACHE) &&
> > > + f2fs_compressed_file(page->mapping->host))
> > > + f2fs_invalidate_compress_pages(F2FS_P_SB(page),
> > > + page->mapping->host->i_ino);
> > > +
> > > clear_cold_data(page);
> > > f2fs_clear_page_private(page);
> > > return 1;
> > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > index 8b5975731439..a44dfc9a594a 100644
> > > --- a/fs/f2fs/super.c
> > > +++ b/fs/f2fs/super.c
> > > @@ -1915,6 +1915,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> > > bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT);
> > > bool no_io_align = !F2FS_IO_ALIGNED(sbi);
> > > bool no_atgc = !test_opt(sbi, ATGC);
> > > + bool no_compress_cache = !test_opt(sbi, COMPRESS_CACHE);
> > > bool checkpoint_changed;
> > > #ifdef CONFIG_QUOTA
> > > int i, j;
> > > @@ -2007,6 +2008,12 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> > > goto restore_opts;
> > > }
> > >
> > > + if (no_compress_cache == !!test_opt(sbi, COMPRESS_CACHE)) {
> > > + err = -EINVAL;
> > > + f2fs_warn(sbi, "switch compress_cache option is not allowed");
> > > + goto restore_opts;
> > > + }
> > > +
> > > if ((*flags & SB_RDONLY) && test_opt(sbi, DISABLE_CHECKPOINT)) {
> > > err = -EINVAL;
> > > f2fs_warn(sbi, "disabling checkpoint not compatible with read-only");
> > > --
> > > 2.29.2
> > >
> > >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > Linux-f2fs-devel mailing list
> > > > > > > [email protected]
> > > > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > > > > .
> > > > > > >
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > Linux-f2fs-devel mailing list
> > > > > > [email protected]
> > > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > > > .
> > > > > >
> > > > .
> > > >
> > .
> >

2021-01-14 02:47:23

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/13 23:41, Jaegeuk Kim wrote:
> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> [58691.077338] ------------[ cut here ]------------
> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [58691.176163] Call Trace:
> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> [58691.180549] ? newidle_balance+0x253/0x3d0
> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> [58691.192489] process_one_work+0x221/0x3a0
> [58691.194482] worker_thread+0x4d/0x3f0
> [58691.198867] kthread+0x114/0x150
> [58691.202243] ? process_one_work+0x3a0/0x3a0
> [58691.205367] ? kthread_park+0x90/0x90
> [58691.208244] ret_from_fork+0x22/0x30

Below patch fixes two issues, I expect this can fix above warning at least.

- detect truncation during f2fs_cache_compressed_page()
- don't set PageUptodate for temporary page in f2fs_load_compressed_page()

From: Chao Yu <[email protected]>

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/compress.c | 20 +++++++++++++-------
fs/f2fs/data.c | 3 +--
fs/f2fs/f2fs.h | 6 +++---
3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 0fec71e40001..f364c10c506c 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
if (!test_opt(sbi, COMPRESS_CACHE))
return;

- if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
+ if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
return;

si_meminfo(&si);
@@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
return;
}

- memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
- SetPageUptodate(cpage);
-
f2fs_set_page_private(cpage, ino);

+ if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
+ goto out;
+
+ memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
+ SetPageUptodate(cpage);
+out:
f2fs_put_page(cpage, 1);
}

-void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
block_t blkaddr)
{
struct page *cpage;
+ bool hitted = false;

if (!test_opt(sbi, COMPRESS_CACHE))
- return;
+ return false;

cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
@@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
atomic_inc(&sbi->compress_page_hit);
memcpy(page_address(page),
page_address(cpage), PAGE_SIZE);
- SetPageUptodate(page);
+ hitted = true;
}
f2fs_put_page(cpage, 1);
}
+
+ return hitted;
}

void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b3973494b102..3705c272b76a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
blkaddr = data_blkaddr(dn.inode, dn.node_page,
dn.ofs_in_node + i + 1);

- f2fs_load_compressed_page(sbi, page, blkaddr);
- if (PageUptodate(page)) {
+ if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
if (atomic_dec_and_test(&dic->remaining_pages))
f2fs_decompress_cluster(dic);
continue;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9f79a6825f06..b807970d67b1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
nid_t ino, block_t blkaddr);
-void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
block_t blkaddr);
void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
#else
@@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
block_t blkaddr) { }
static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
struct page *page, nid_t ino, block_t blkaddr) { }
-static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
- struct page *page, block_t blkaddr) { }
+static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
+ struct page *page, block_t blkaddr) { return false; }
static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
nid_t ino) { }
#endif
--
2.29.2

2021-01-14 04:09:01

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/14, Chao Yu wrote:
> On 2021/1/13 23:41, Jaegeuk Kim wrote:
> > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > [58691.077338] ------------[ cut here ]------------
> > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [58691.176163] Call Trace:
> > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > [58691.180549] ? newidle_balance+0x253/0x3d0
> > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > [58691.192489] process_one_work+0x221/0x3a0
> > [58691.194482] worker_thread+0x4d/0x3f0
> > [58691.198867] kthread+0x114/0x150
> > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > [58691.205367] ? kthread_park+0x90/0x90
> > [58691.208244] ret_from_fork+0x22/0x30
>
> Below patch fixes two issues, I expect this can fix above warning at least.

[106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
[106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
[106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
[106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
[106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
[106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
[106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
[106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
[106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
[106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
[106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
[106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
[106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
[106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[106115.635587] Call Trace:
[106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
[106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
[106115.635587] read_pages+0x8e/0x280
[106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
[106115.665909] do_page_cache_ra+0x3d/0x40
[106115.670756] ondemand_readahead+0x2c1/0x2e0
[106115.671682] page_cache_sync_ra+0xd4/0xe0
[106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
[106115.679158] generic_file_buffered_read+0x113/0x4a0
[106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
[106115.685672] ? __mark_inode_dirty+0x98/0x330
[106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
[106115.691168] generic_file_read_iter+0xdf/0x140
[106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
[106115.699450] aio_read+0xef/0x1b0
[106115.699450] ? do_user_addr_fault+0x1b8/0x450
[106115.699450] io_submit_one+0x217/0xbc0
[106115.699450] ? io_submit_one+0x217/0xbc0
[106115.699450] __x64_sys_io_submit+0x8d/0x180
[106115.699450] ? __x64_sys_io_submit+0x8d/0x180
[106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
[106115.717468] do_syscall_64+0x38/0x90
[106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9

>
> - detect truncation during f2fs_cache_compressed_page()
> - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
>
> From: Chao Yu <[email protected]>
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/compress.c | 20 +++++++++++++-------
> fs/f2fs/data.c | 3 +--
> fs/f2fs/f2fs.h | 6 +++---
> 3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 0fec71e40001..f364c10c506c 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> if (!test_opt(sbi, COMPRESS_CACHE))
> return;
>
> - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> return;
>
> si_meminfo(&si);
> @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> return;
> }
>
> - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> - SetPageUptodate(cpage);
> -
> f2fs_set_page_private(cpage, ino);
>
> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> + goto out;
> +
> + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> + SetPageUptodate(cpage);
> +out:
> f2fs_put_page(cpage, 1);
> }
>
> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> block_t blkaddr)
> {
> struct page *cpage;
> + bool hitted = false;
>
> if (!test_opt(sbi, COMPRESS_CACHE))
> - return;
> + return false;
>
> cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> atomic_inc(&sbi->compress_page_hit);
> memcpy(page_address(page),
> page_address(cpage), PAGE_SIZE);
> - SetPageUptodate(page);
> + hitted = true;
> }
> f2fs_put_page(cpage, 1);
> }
> +
> + return hitted;
> }
>
> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index b3973494b102..3705c272b76a 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> blkaddr = data_blkaddr(dn.inode, dn.node_page,
> dn.ofs_in_node + i + 1);
>
> - f2fs_load_compressed_page(sbi, page, blkaddr);
> - if (PageUptodate(page)) {
> + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> if (atomic_dec_and_test(&dic->remaining_pages))
> f2fs_decompress_cluster(dic);
> continue;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9f79a6825f06..b807970d67b1 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
> void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> nid_t ino, block_t blkaddr);
> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> block_t blkaddr);
> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> #else
> @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> block_t blkaddr) { }
> static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> struct page *page, nid_t ino, block_t blkaddr) { }
> -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> - struct page *page, block_t blkaddr) { }
> +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> + struct page *page, block_t blkaddr) { return false; }
> static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> nid_t ino) { }
> #endif
> --
> 2.29.2

2021-01-15 09:36:07

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/14 12:06, Jaegeuk Kim wrote:
> On 01/14, Chao Yu wrote:
>> On 2021/1/13 23:41, Jaegeuk Kim wrote:
>>> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
>>> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
>>> [58691.077338] ------------[ cut here ]------------
>>> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
>>> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
>>> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
>>> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
>>> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
>>> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
>>> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
>>> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
>>> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
>>> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
>>> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
>>> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
>>> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [58691.176163] Call Trace:
>>> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
>>> [58691.180549] ? newidle_balance+0x253/0x3d0
>>> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
>>> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
>>> [58691.192489] process_one_work+0x221/0x3a0
>>> [58691.194482] worker_thread+0x4d/0x3f0
>>> [58691.198867] kthread+0x114/0x150
>>> [58691.202243] ? process_one_work+0x3a0/0x3a0
>>> [58691.205367] ? kthread_park+0x90/0x90
>>> [58691.208244] ret_from_fork+0x22/0x30
>>
>> Below patch fixes two issues, I expect this can fix above warning at least.
>
> [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]

Jaegeuk,

Could you please help to run:

gdb f2fs.ko
(gdb) l *(f2fs_read_multi_pages+0x415)

to see where we hit the panic.

Thanks,

> [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
> [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
> [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
> [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
> [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
> [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
> [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
> [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
> [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
> [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [106115.635587] Call Trace:
> [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
> [106115.635587] read_pages+0x8e/0x280
> [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
> [106115.665909] do_page_cache_ra+0x3d/0x40
> [106115.670756] ondemand_readahead+0x2c1/0x2e0
> [106115.671682] page_cache_sync_ra+0xd4/0xe0
> [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
> [106115.679158] generic_file_buffered_read+0x113/0x4a0
> [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
> [106115.685672] ? __mark_inode_dirty+0x98/0x330
> [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
> [106115.691168] generic_file_read_iter+0xdf/0x140
> [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> [106115.699450] aio_read+0xef/0x1b0
> [106115.699450] ? do_user_addr_fault+0x1b8/0x450
> [106115.699450] io_submit_one+0x217/0xbc0
> [106115.699450] ? io_submit_one+0x217/0xbc0
> [106115.699450] __x64_sys_io_submit+0x8d/0x180
> [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
> [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
> [106115.717468] do_syscall_64+0x38/0x90
> [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
>>
>> - detect truncation during f2fs_cache_compressed_page()
>> - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
>>
>> From: Chao Yu <[email protected]>
>>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> fs/f2fs/compress.c | 20 +++++++++++++-------
>> fs/f2fs/data.c | 3 +--
>> fs/f2fs/f2fs.h | 6 +++---
>> 3 files changed, 17 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 0fec71e40001..f364c10c506c 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> if (!test_opt(sbi, COMPRESS_CACHE))
>> return;
>>
>> - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>> return;
>>
>> si_meminfo(&si);
>> @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> return;
>> }
>>
>> - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>> - SetPageUptodate(cpage);
>> -
>> f2fs_set_page_private(cpage, ino);
>>
>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>> + goto out;
>> +
>> + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>> + SetPageUptodate(cpage);
>> +out:
>> f2fs_put_page(cpage, 1);
>> }
>>
>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> block_t blkaddr)
>> {
>> struct page *cpage;
>> + bool hitted = false;
>>
>> if (!test_opt(sbi, COMPRESS_CACHE))
>> - return;
>> + return false;
>>
>> cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
>> blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
>> @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> atomic_inc(&sbi->compress_page_hit);
>> memcpy(page_address(page),
>> page_address(cpage), PAGE_SIZE);
>> - SetPageUptodate(page);
>> + hitted = true;
>> }
>> f2fs_put_page(cpage, 1);
>> }
>> +
>> + return hitted;
>> }
>>
>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index b3973494b102..3705c272b76a 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
>> blkaddr = data_blkaddr(dn.inode, dn.node_page,
>> dn.ofs_in_node + i + 1);
>>
>> - f2fs_load_compressed_page(sbi, page, blkaddr);
>> - if (PageUptodate(page)) {
>> + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
>> if (atomic_dec_and_test(&dic->remaining_pages))
>> f2fs_decompress_cluster(dic);
>> continue;
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 9f79a6825f06..b807970d67b1 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
>> void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
>> void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> nid_t ino, block_t blkaddr);
>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>> block_t blkaddr);
>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
>> #else
>> @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
>> block_t blkaddr) { }
>> static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
>> struct page *page, nid_t ino, block_t blkaddr) { }
>> -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>> - struct page *page, block_t blkaddr) { }
>> +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>> + struct page *page, block_t blkaddr) { return false; }
>> static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>> nid_t ino) { }
>> #endif
>> --
>> 2.29.2
> .
>

2021-01-15 15:03:27

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/15, Chao Yu wrote:
> On 2021/1/14 12:06, Jaegeuk Kim wrote:
> > On 01/14, Chao Yu wrote:
> > > On 2021/1/13 23:41, Jaegeuk Kim wrote:
> > > > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > > > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > > > [58691.077338] ------------[ cut here ]------------
> > > > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > > > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > > > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > > > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > > > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > > > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > > > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > > > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > > > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > > > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > > > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > > > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > > > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [58691.176163] Call Trace:
> > > > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > > > [58691.180549] ? newidle_balance+0x253/0x3d0
> > > > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > > > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > > > [58691.192489] process_one_work+0x221/0x3a0
> > > > [58691.194482] worker_thread+0x4d/0x3f0
> > > > [58691.198867] kthread+0x114/0x150
> > > > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > > > [58691.205367] ? kthread_park+0x90/0x90
> > > > [58691.208244] ret_from_fork+0x22/0x30
> > >
> > > Below patch fixes two issues, I expect this can fix above warning at least.
> >
> > [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
>
> Jaegeuk,
>
> Could you please help to run:
>
> gdb f2fs.ko
> (gdb) l *(f2fs_read_multi_pages+0x415)
>
> to see where we hit the panic.

It's fs/f2fs/data.c:2203

2199 goto out_put_dnode;
2200 }
2201
2202 for (i = 0; i < dic->nr_cpages; i++) {
2203 struct page *page = dic->cpages[i];
2204 block_t blkaddr;
2205 struct bio_post_read_ctx *ctx;
2206
2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
2208 dn.ofs_in_node + i + 1);


>
> Thanks,
>
> > [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
> > [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
> > [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
> > [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
> > [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
> > [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
> > [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
> > [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
> > [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
> > [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [106115.635587] Call Trace:
> > [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
> > [106115.635587] read_pages+0x8e/0x280
> > [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
> > [106115.665909] do_page_cache_ra+0x3d/0x40
> > [106115.670756] ondemand_readahead+0x2c1/0x2e0
> > [106115.671682] page_cache_sync_ra+0xd4/0xe0
> > [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
> > [106115.679158] generic_file_buffered_read+0x113/0x4a0
> > [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
> > [106115.685672] ? __mark_inode_dirty+0x98/0x330
> > [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
> > [106115.691168] generic_file_read_iter+0xdf/0x140
> > [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > [106115.699450] aio_read+0xef/0x1b0
> > [106115.699450] ? do_user_addr_fault+0x1b8/0x450
> > [106115.699450] io_submit_one+0x217/0xbc0
> > [106115.699450] ? io_submit_one+0x217/0xbc0
> > [106115.699450] __x64_sys_io_submit+0x8d/0x180
> > [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
> > [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
> > [106115.717468] do_syscall_64+0x38/0x90
> > [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > >
> > > - detect truncation during f2fs_cache_compressed_page()
> > > - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
> > >
> > > From: Chao Yu <[email protected]>
> > >
> > > Signed-off-by: Chao Yu <[email protected]>
> > > ---
> > > fs/f2fs/compress.c | 20 +++++++++++++-------
> > > fs/f2fs/data.c | 3 +--
> > > fs/f2fs/f2fs.h | 6 +++---
> > > 3 files changed, 17 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > index 0fec71e40001..f364c10c506c 100644
> > > --- a/fs/f2fs/compress.c
> > > +++ b/fs/f2fs/compress.c
> > > @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > return;
> > >
> > > - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > return;
> > >
> > > si_meminfo(&si);
> > > @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > return;
> > > }
> > >
> > > - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > - SetPageUptodate(cpage);
> > > -
> > > f2fs_set_page_private(cpage, ino);
> > >
> > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > + goto out;
> > > +
> > > + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > + SetPageUptodate(cpage);
> > > +out:
> > > f2fs_put_page(cpage, 1);
> > > }
> > >
> > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > block_t blkaddr)
> > > {
> > > struct page *cpage;
> > > + bool hitted = false;
> > >
> > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > - return;
> > > + return false;
> > >
> > > cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> > > blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> > > @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > atomic_inc(&sbi->compress_page_hit);
> > > memcpy(page_address(page),
> > > page_address(cpage), PAGE_SIZE);
> > > - SetPageUptodate(page);
> > > + hitted = true;
> > > }
> > > f2fs_put_page(cpage, 1);
> > > }
> > > +
> > > + return hitted;
> > > }
> > >
> > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index b3973494b102..3705c272b76a 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> > > blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > > dn.ofs_in_node + i + 1);
> > >
> > > - f2fs_load_compressed_page(sbi, page, blkaddr);
> > > - if (PageUptodate(page)) {
> > > + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> > > if (atomic_dec_and_test(&dic->remaining_pages))
> > > f2fs_decompress_cluster(dic);
> > > continue;
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 9f79a6825f06..b807970d67b1 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> > > void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
> > > void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > nid_t ino, block_t blkaddr);
> > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > block_t blkaddr);
> > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> > > #else
> > > @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> > > block_t blkaddr) { }
> > > static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> > > struct page *page, nid_t ino, block_t blkaddr) { }
> > > -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> > > - struct page *page, block_t blkaddr) { }
> > > +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> > > + struct page *page, block_t blkaddr) { return false; }
> > > static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> > > nid_t ino) { }
> > > #endif
> > > --
> > > 2.29.2
> > .
> >

2021-01-16 12:11:10

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/15 22:59, Jaegeuk Kim wrote:
> On 01/15, Chao Yu wrote:
>> On 2021/1/14 12:06, Jaegeuk Kim wrote:
>>> On 01/14, Chao Yu wrote:
>>>> On 2021/1/13 23:41, Jaegeuk Kim wrote:
>>>>> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
>>>>> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
>>>>> [58691.077338] ------------[ cut here ]------------
>>>>> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>>>> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
>>>>> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
>>>>> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>>>> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
>>>>> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>>>> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
>>>>> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
>>>>> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
>>>>> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
>>>>> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
>>>>> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
>>>>> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
>>>>> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
>>>>> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
>>>>> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>>> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>>>> [58691.176163] Call Trace:
>>>>> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
>>>>> [58691.180549] ? newidle_balance+0x253/0x3d0
>>>>> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
>>>>> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
>>>>> [58691.192489] process_one_work+0x221/0x3a0
>>>>> [58691.194482] worker_thread+0x4d/0x3f0
>>>>> [58691.198867] kthread+0x114/0x150
>>>>> [58691.202243] ? process_one_work+0x3a0/0x3a0
>>>>> [58691.205367] ? kthread_park+0x90/0x90
>>>>> [58691.208244] ret_from_fork+0x22/0x30
>>>>
>>>> Below patch fixes two issues, I expect this can fix above warning at least.
>>>
>>> [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
>>> [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
>>> [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>> [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
>>
>> Jaegeuk,
>>
>> Could you please help to run:
>>
>> gdb f2fs.ko
>> (gdb) l *(f2fs_read_multi_pages+0x415)
>>
>> to see where we hit the panic.
>
> It's fs/f2fs/data.c:2203
>
> 2199 goto out_put_dnode;
> 2200 }
> 2201
> 2202 for (i = 0; i < dic->nr_cpages; i++) {

I doubt when i == cc->nr_cpages, dic was released in below condition,
then dic->nr_cpages can be any value (may be larger than cc->nr_cpages),
then we may continue the loop, and will access invalid pointer dic.

if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
if (atomic_dec_and_test(&dic->remaining_pages))
f2fs_decompress_cluster(dic);
continue;
}


I'd like to add a condition here (in between line 2202 and line 2203) to make
sure whether this can happen, could you please help to verify this?

f2fs_bug_on(sbi, i >= cc->nr_cpages);

Thanks,

> 2203 struct page *page = dic->cpages[i];
> 2204 block_t blkaddr;
> 2205 struct bio_post_read_ctx *ctx;
> 2206
> 2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
> 2208 dn.ofs_in_node + i + 1);
>
>
>>
>> Thanks,
>>
>>> [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
>>> [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
>>> [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
>>> [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
>>> [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
>>> [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
>>> [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
>>> [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
>>> [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
>>> [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [106115.635587] Call Trace:
>>> [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
>>> [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
>>> [106115.635587] read_pages+0x8e/0x280
>>> [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
>>> [106115.665909] do_page_cache_ra+0x3d/0x40
>>> [106115.670756] ondemand_readahead+0x2c1/0x2e0
>>> [106115.671682] page_cache_sync_ra+0xd4/0xe0
>>> [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
>>> [106115.679158] generic_file_buffered_read+0x113/0x4a0
>>> [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
>>> [106115.685672] ? __mark_inode_dirty+0x98/0x330
>>> [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
>>> [106115.691168] generic_file_read_iter+0xdf/0x140
>>> [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
>>> [106115.699450] aio_read+0xef/0x1b0
>>> [106115.699450] ? do_user_addr_fault+0x1b8/0x450
>>> [106115.699450] io_submit_one+0x217/0xbc0
>>> [106115.699450] ? io_submit_one+0x217/0xbc0
>>> [106115.699450] __x64_sys_io_submit+0x8d/0x180
>>> [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
>>> [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
>>> [106115.717468] do_syscall_64+0x38/0x90
>>> [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>
>>>>
>>>> - detect truncation during f2fs_cache_compressed_page()
>>>> - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
>>>>
>>>> From: Chao Yu <[email protected]>
>>>>
>>>> Signed-off-by: Chao Yu <[email protected]>
>>>> ---
>>>> fs/f2fs/compress.c | 20 +++++++++++++-------
>>>> fs/f2fs/data.c | 3 +--
>>>> fs/f2fs/f2fs.h | 6 +++---
>>>> 3 files changed, 17 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>> index 0fec71e40001..f364c10c506c 100644
>>>> --- a/fs/f2fs/compress.c
>>>> +++ b/fs/f2fs/compress.c
>>>> @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> if (!test_opt(sbi, COMPRESS_CACHE))
>>>> return;
>>>>
>>>> - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
>>>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>>>> return;
>>>>
>>>> si_meminfo(&si);
>>>> @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> return;
>>>> }
>>>>
>>>> - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>>>> - SetPageUptodate(cpage);
>>>> -
>>>> f2fs_set_page_private(cpage, ino);
>>>>
>>>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>>>> + goto out;
>>>> +
>>>> + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>>>> + SetPageUptodate(cpage);
>>>> +out:
>>>> f2fs_put_page(cpage, 1);
>>>> }
>>>>
>>>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> block_t blkaddr)
>>>> {
>>>> struct page *cpage;
>>>> + bool hitted = false;
>>>>
>>>> if (!test_opt(sbi, COMPRESS_CACHE))
>>>> - return;
>>>> + return false;
>>>>
>>>> cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
>>>> blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
>>>> @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> atomic_inc(&sbi->compress_page_hit);
>>>> memcpy(page_address(page),
>>>> page_address(cpage), PAGE_SIZE);
>>>> - SetPageUptodate(page);
>>>> + hitted = true;
>>>> }
>>>> f2fs_put_page(cpage, 1);
>>>> }
>>>> +
>>>> + return hitted;
>>>> }
>>>>
>>>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index b3973494b102..3705c272b76a 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
>>>> blkaddr = data_blkaddr(dn.inode, dn.node_page,
>>>> dn.ofs_in_node + i + 1);
>>>>
>>>> - f2fs_load_compressed_page(sbi, page, blkaddr);
>>>> - if (PageUptodate(page)) {
>>>> + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
>>>> if (atomic_dec_and_test(&dic->remaining_pages))
>>>> f2fs_decompress_cluster(dic);
>>>> continue;
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 9f79a6825f06..b807970d67b1 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
>>>> void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
>>>> void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> nid_t ino, block_t blkaddr);
>>>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>> block_t blkaddr);
>>>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
>>>> #else
>>>> @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
>>>> block_t blkaddr) { }
>>>> static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
>>>> struct page *page, nid_t ino, block_t blkaddr) { }
>>>> -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>>> - struct page *page, block_t blkaddr) { }
>>>> +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>>> + struct page *page, block_t blkaddr) { return false; }
>>>> static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>>>> nid_t ino) { }
>>>> #endif
>>>> --
>>>> 2.29.2
>>> .
>>>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>

2021-01-19 21:48:37

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/16, Chao Yu wrote:
> On 2021/1/15 22:59, Jaegeuk Kim wrote:
> > On 01/15, Chao Yu wrote:
> > > On 2021/1/14 12:06, Jaegeuk Kim wrote:
> > > > On 01/14, Chao Yu wrote:
> > > > > On 2021/1/13 23:41, Jaegeuk Kim wrote:
> > > > > > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > > > > > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > > > > > [58691.077338] ------------[ cut here ]------------
> > > > > > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > > > > > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > > > > > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > > > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > > > > > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > > > > > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > > > > > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > > > > > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > > > > > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > > > > > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > > > > > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > > > > > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > > > > > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > > > > > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > [58691.176163] Call Trace:
> > > > > > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > > > > > [58691.180549] ? newidle_balance+0x253/0x3d0
> > > > > > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > > > > > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > > > > > [58691.192489] process_one_work+0x221/0x3a0
> > > > > > [58691.194482] worker_thread+0x4d/0x3f0
> > > > > > [58691.198867] kthread+0x114/0x150
> > > > > > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > > > > > [58691.205367] ? kthread_park+0x90/0x90
> > > > > > [58691.208244] ret_from_fork+0x22/0x30
> > > > >
> > > > > Below patch fixes two issues, I expect this can fix above warning at least.
> > > >
> > > > [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > > > [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > > > [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
> > >
> > > Jaegeuk,
> > >
> > > Could you please help to run:
> > >
> > > gdb f2fs.ko
> > > (gdb) l *(f2fs_read_multi_pages+0x415)
> > >
> > > to see where we hit the panic.
> >
> > It's fs/f2fs/data.c:2203
> >
> > 2199 goto out_put_dnode;
> > 2200 }
> > 2201
> > 2202 for (i = 0; i < dic->nr_cpages; i++) {
>
> I doubt when i == cc->nr_cpages, dic was released in below condition,
> then dic->nr_cpages can be any value (may be larger than cc->nr_cpages),
> then we may continue the loop, and will access invalid pointer dic.
>
> if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> if (atomic_dec_and_test(&dic->remaining_pages))
> f2fs_decompress_cluster(dic);
> continue;
> }
>
>
> I'd like to add a condition here (in between line 2202 and line 2203) to
> make sure whether this can happen, could you please help to verify this?
>
> f2fs_bug_on(sbi, i >= cc->nr_cpages);

No, it seems this is not the case.

>
> Thanks,
>
> > 2203 struct page *page = dic->cpages[i];
> > 2204 block_t blkaddr;
> > 2205 struct bio_post_read_ctx *ctx;
> > 2206
> > 2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > 2208 dn.ofs_in_node + i + 1);
> >
> >
> > >
> > > Thanks,
> > >
> > > > [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
> > > > [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
> > > > [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
> > > > [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
> > > > [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
> > > > [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
> > > > [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
> > > > [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
> > > > [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
> > > > [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [106115.635587] Call Trace:
> > > > [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > > > [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
> > > > [106115.635587] read_pages+0x8e/0x280
> > > > [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
> > > > [106115.665909] do_page_cache_ra+0x3d/0x40
> > > > [106115.670756] ondemand_readahead+0x2c1/0x2e0
> > > > [106115.671682] page_cache_sync_ra+0xd4/0xe0
> > > > [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
> > > > [106115.679158] generic_file_buffered_read+0x113/0x4a0
> > > > [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
> > > > [106115.685672] ? __mark_inode_dirty+0x98/0x330
> > > > [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
> > > > [106115.691168] generic_file_read_iter+0xdf/0x140
> > > > [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > > > [106115.699450] aio_read+0xef/0x1b0
> > > > [106115.699450] ? do_user_addr_fault+0x1b8/0x450
> > > > [106115.699450] io_submit_one+0x217/0xbc0
> > > > [106115.699450] ? io_submit_one+0x217/0xbc0
> > > > [106115.699450] __x64_sys_io_submit+0x8d/0x180
> > > > [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
> > > > [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
> > > > [106115.717468] do_syscall_64+0x38/0x90
> > > > [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > >
> > > > >
> > > > > - detect truncation during f2fs_cache_compressed_page()
> > > > > - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
> > > > >
> > > > > From: Chao Yu <[email protected]>
> > > > >
> > > > > Signed-off-by: Chao Yu <[email protected]>
> > > > > ---
> > > > > fs/f2fs/compress.c | 20 +++++++++++++-------
> > > > > fs/f2fs/data.c | 3 +--
> > > > > fs/f2fs/f2fs.h | 6 +++---
> > > > > 3 files changed, 17 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > > index 0fec71e40001..f364c10c506c 100644
> > > > > --- a/fs/f2fs/compress.c
> > > > > +++ b/fs/f2fs/compress.c
> > > > > @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > return;
> > > > >
> > > > > - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > return;
> > > > >
> > > > > si_meminfo(&si);
> > > > > @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > return;
> > > > > }
> > > > >
> > > > > - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > - SetPageUptodate(cpage);
> > > > > -
> > > > > f2fs_set_page_private(cpage, ino);
> > > > >
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > + goto out;
> > > > > +
> > > > > + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > + SetPageUptodate(cpage);
> > > > > +out:
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > >
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > block_t blkaddr)
> > > > > {
> > > > > struct page *cpage;
> > > > > + bool hitted = false;
> > > > >
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > - return;
> > > > > + return false;
> > > > >
> > > > > cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> > > > > blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> > > > > @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > atomic_inc(&sbi->compress_page_hit);
> > > > > memcpy(page_address(page),
> > > > > page_address(cpage), PAGE_SIZE);
> > > > > - SetPageUptodate(page);
> > > > > + hitted = true;
> > > > > }
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > > +
> > > > > + return hitted;
> > > > > }
> > > > >
> > > > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > > index b3973494b102..3705c272b76a 100644
> > > > > --- a/fs/f2fs/data.c
> > > > > +++ b/fs/f2fs/data.c
> > > > > @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> > > > > blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > > > > dn.ofs_in_node + i + 1);
> > > > >
> > > > > - f2fs_load_compressed_page(sbi, page, blkaddr);
> > > > > - if (PageUptodate(page)) {
> > > > > + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> > > > > if (atomic_dec_and_test(&dic->remaining_pages))
> > > > > f2fs_decompress_cluster(dic);
> > > > > continue;
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index 9f79a6825f06..b807970d67b1 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> > > > > void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
> > > > > void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > nid_t ino, block_t blkaddr);
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > block_t blkaddr);
> > > > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> > > > > #else
> > > > > @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> > > > > block_t blkaddr) { }
> > > > > static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> > > > > struct page *page, nid_t ino, block_t blkaddr) { }
> > > > > -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> > > > > - struct page *page, block_t blkaddr) { }
> > > > > +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> > > > > + struct page *page, block_t blkaddr) { return false; }
> > > > > static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> > > > > nid_t ino) { }
> > > > > #endif
> > > > > --
> > > > > 2.29.2
> > > > .
> > > >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >

2021-01-22 02:40:04

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/20 5:42, Jaegeuk Kim wrote:
> On 01/16, Chao Yu wrote:
>> On 2021/1/15 22:59, Jaegeuk Kim wrote:
>>> On 01/15, Chao Yu wrote:
>>>> On 2021/1/14 12:06, Jaegeuk Kim wrote:
>>>>> On 01/14, Chao Yu wrote:
>>>>>> On 2021/1/13 23:41, Jaegeuk Kim wrote:
>>>>>>> [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
>>>>>>> [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
>>>>>>> [58691.077338] ------------[ cut here ]------------
>>>>>>> [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>>>>>> [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
>>>>>>> [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
>>>>>>> [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>>>>>> [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
>>>>>>> [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
>>>>>>> [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
>>>>>>> [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
>>>>>>> [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
>>>>>>> [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
>>>>>>> [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
>>>>>>> [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
>>>>>>> [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
>>>>>>> [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
>>>>>>> [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>>>> [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
>>>>>>> [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>>>>> [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>>>>>> [58691.176163] Call Trace:
>>>>>>> [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
>>>>>>> [58691.180549] ? newidle_balance+0x253/0x3d0
>>>>>>> [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
>>>>>>> [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
>>>>>>> [58691.192489] process_one_work+0x221/0x3a0
>>>>>>> [58691.194482] worker_thread+0x4d/0x3f0
>>>>>>> [58691.198867] kthread+0x114/0x150
>>>>>>> [58691.202243] ? process_one_work+0x3a0/0x3a0
>>>>>>> [58691.205367] ? kthread_park+0x90/0x90
>>>>>>> [58691.208244] ret_from_fork+0x22/0x30
>>>>>>
>>>>>> Below patch fixes two issues, I expect this can fix above warning at least.
>>>>>
>>>>> [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
>>>>> [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
>>>>> [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
>>>>> [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
>>>>
>>>> Jaegeuk,
>>>>
>>>> Could you please help to run:
>>>>
>>>> gdb f2fs.ko
>>>> (gdb) l *(f2fs_read_multi_pages+0x415)
>>>>
>>>> to see where we hit the panic.
>>>
>>> It's fs/f2fs/data.c:2203
>>>
>>> 2199 goto out_put_dnode;
>>> 2200 }
>>> 2201
>>> 2202 for (i = 0; i < dic->nr_cpages; i++) {
>>
>> I doubt when i == cc->nr_cpages, dic was released in below condition,
>> then dic->nr_cpages can be any value (may be larger than cc->nr_cpages),
>> then we may continue the loop, and will access invalid pointer dic.
>>
>> if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
>> if (atomic_dec_and_test(&dic->remaining_pages))
>> f2fs_decompress_cluster(dic);
>> continue;
>> }
>>
>>
>> I'd like to add a condition here (in between line 2202 and line 2203) to
>> make sure whether this can happen, could you please help to verify this?
>>
>> f2fs_bug_on(sbi, i >= cc->nr_cpages);
>
> No, it seems this is not the case.

Oops, could you please help to remove all below codes and do the test again
to check whether they are the buggy codes? as I doubt there is use-after-free
bug.

if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
if (atomic_dec_and_test(&dic->remaining_pages))
f2fs_decompress_cluster(dic);
continue;
}

Before that, please merge last fixing patch as below:

https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/commit/?h=dev&id=2371a9b8131712276458f1991a6ae394ef6c6c90

Thanks,

>
>>
>> Thanks,
>>
>>> 2203 struct page *page = dic->cpages[i];
>>> 2204 block_t blkaddr;
>>> 2205 struct bio_post_read_ctx *ctx;
>>> 2206
>>> 2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
>>> 2208 dn.ofs_in_node + i + 1);
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>> [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
>>>>> [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
>>>>> [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
>>>>> [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
>>>>> [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
>>>>> [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
>>>>> [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
>>>>> [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
>>>>> [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>> [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
>>>>> [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>>> [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>>>> [106115.635587] Call Trace:
>>>>> [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
>>>>> [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
>>>>> [106115.635587] read_pages+0x8e/0x280
>>>>> [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
>>>>> [106115.665909] do_page_cache_ra+0x3d/0x40
>>>>> [106115.670756] ondemand_readahead+0x2c1/0x2e0
>>>>> [106115.671682] page_cache_sync_ra+0xd4/0xe0
>>>>> [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
>>>>> [106115.679158] generic_file_buffered_read+0x113/0x4a0
>>>>> [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
>>>>> [106115.685672] ? __mark_inode_dirty+0x98/0x330
>>>>> [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
>>>>> [106115.691168] generic_file_read_iter+0xdf/0x140
>>>>> [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
>>>>> [106115.699450] aio_read+0xef/0x1b0
>>>>> [106115.699450] ? do_user_addr_fault+0x1b8/0x450
>>>>> [106115.699450] io_submit_one+0x217/0xbc0
>>>>> [106115.699450] ? io_submit_one+0x217/0xbc0
>>>>> [106115.699450] __x64_sys_io_submit+0x8d/0x180
>>>>> [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
>>>>> [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
>>>>> [106115.717468] do_syscall_64+0x38/0x90
>>>>> [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>>>>
>>>>>>
>>>>>> - detect truncation during f2fs_cache_compressed_page()
>>>>>> - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
>>>>>>
>>>>>> From: Chao Yu <[email protected]>
>>>>>>
>>>>>> Signed-off-by: Chao Yu <[email protected]>
>>>>>> ---
>>>>>> fs/f2fs/compress.c | 20 +++++++++++++-------
>>>>>> fs/f2fs/data.c | 3 +--
>>>>>> fs/f2fs/f2fs.h | 6 +++---
>>>>>> 3 files changed, 17 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>>>> index 0fec71e40001..f364c10c506c 100644
>>>>>> --- a/fs/f2fs/compress.c
>>>>>> +++ b/fs/f2fs/compress.c
>>>>>> @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> if (!test_opt(sbi, COMPRESS_CACHE))
>>>>>> return;
>>>>>>
>>>>>> - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
>>>>>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>>>>>> return;
>>>>>>
>>>>>> si_meminfo(&si);
>>>>>> @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> return;
>>>>>> }
>>>>>>
>>>>>> - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>>>>>> - SetPageUptodate(cpage);
>>>>>> -
>>>>>> f2fs_set_page_private(cpage, ino);
>>>>>>
>>>>>> + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>>>>>> + goto out;
>>>>>> +
>>>>>> + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
>>>>>> + SetPageUptodate(cpage);
>>>>>> +out:
>>>>>> f2fs_put_page(cpage, 1);
>>>>>> }
>>>>>>
>>>>>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> block_t blkaddr)
>>>>>> {
>>>>>> struct page *cpage;
>>>>>> + bool hitted = false;
>>>>>>
>>>>>> if (!test_opt(sbi, COMPRESS_CACHE))
>>>>>> - return;
>>>>>> + return false;
>>>>>>
>>>>>> cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
>>>>>> blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
>>>>>> @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> atomic_inc(&sbi->compress_page_hit);
>>>>>> memcpy(page_address(page),
>>>>>> page_address(cpage), PAGE_SIZE);
>>>>>> - SetPageUptodate(page);
>>>>>> + hitted = true;
>>>>>> }
>>>>>> f2fs_put_page(cpage, 1);
>>>>>> }
>>>>>> +
>>>>>> + return hitted;
>>>>>> }
>>>>>>
>>>>>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>> index b3973494b102..3705c272b76a 100644
>>>>>> --- a/fs/f2fs/data.c
>>>>>> +++ b/fs/f2fs/data.c
>>>>>> @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
>>>>>> blkaddr = data_blkaddr(dn.inode, dn.node_page,
>>>>>> dn.ofs_in_node + i + 1);
>>>>>>
>>>>>> - f2fs_load_compressed_page(sbi, page, blkaddr);
>>>>>> - if (PageUptodate(page)) {
>>>>>> + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
>>>>>> if (atomic_dec_and_test(&dic->remaining_pages))
>>>>>> f2fs_decompress_cluster(dic);
>>>>>> continue;
>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>> index 9f79a6825f06..b807970d67b1 100644
>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>> @@ -3951,7 +3951,7 @@ struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
>>>>>> void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
>>>>>> void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> nid_t ino, block_t blkaddr);
>>>>>> -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>>>>>> block_t blkaddr);
>>>>>> void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
>>>>>> #else
>>>>>> @@ -3990,8 +3990,8 @@ static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
>>>>>> block_t blkaddr) { }
>>>>>> static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
>>>>>> struct page *page, nid_t ino, block_t blkaddr) { }
>>>>>> -static inline void f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>>>>> - struct page *page, block_t blkaddr) { }
>>>>>> +static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>>>>> + struct page *page, block_t blkaddr) { return false; }
>>>>>> static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
>>>>>> nid_t ino) { }
>>>>>> #endif
>>>>>> --
>>>>>> 2.29.2
>>>>> .
>>>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
> .
>

2021-01-28 02:52:56

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/22 10:17, Chao Yu wrote:
>> No, it seems this is not the case.
> Oops, could you please help to remove all below codes and do the test again
> to check whether they are the buggy codes? as I doubt there is use-after-free
> bug.

Any test result? :)

Thanks,

2021-01-28 16:18:13

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 01/28, Chao Yu wrote:
> On 2021/1/22 10:17, Chao Yu wrote:
> > > No, it seems this is not the case.
> > Oops, could you please help to remove all below codes and do the test again
> > to check whether they are the buggy codes? as I doubt there is use-after-free
> > bug.
>
> Any test result? :)

It seems I don't see the errors anymore. Will you post another version?

>
> Thanks,

2021-01-29 01:15:46

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks

On 2021/1/29 0:12, Jaegeuk Kim wrote:
> On 01/28, Chao Yu wrote:
>> On 2021/1/22 10:17, Chao Yu wrote:
>>>> No, it seems this is not the case.
>>> Oops, could you please help to remove all below codes and do the test again
>>> to check whether they are the buggy codes? as I doubt there is use-after-free
>>> bug.
>>
>> Any test result? :)
>
> It seems I don't see the errors anymore. Will you post another version?

No, your test result only indicate that bug was caused by race condition
of that line with high possibility, however I've no idea how that happen
after several round code review. Still struggling in troubleshooting this
issue.

Thanks,

>
>>
>> Thanks,
> .
>