[f2fs-dev] [PATCH v2 5/5] f2fs: show more debug info for per-temperature log
- Add to account and show per-log dirty_seg, full_seg and valid_blocks
in debugfs.
- reformat printed info.
TYPEsegnosecno zoneno dirty_seg full_seg valid_blk
- COLD data: 1523 1523 1523 1 0399
- WARM data: 769 769 769 20255 133098
- HOTdata: 767 767 767 9 0167
- Dir dnode: 22 22 22 3 0 70
- File dnode: 722 722 722 14 10 6505
- Indir nodes:222 1 0 3
Signed-off-by: Chao Yu
---
v2:
- account debug info for current segments.
fs/f2fs/debug.c | 64 +++--
fs/f2fs/f2fs.h | 3 +++
2 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 0dbcb0f9c019..4276c0f79beb 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -174,6 +174,26 @@ static void update_general_status(struct f2fs_sb_info *sbi)
for (i = META_CP; i < META_MAX; i++)
si->meta_count[i] = atomic_read(>meta_count[i]);
+ for (i = 0; i < NO_CHECK_TYPE; i++) {
+ si->dirty_seg[i] = 0;
+ si->full_seg[i] = 0;
+ si->valid_blks[i] = 0;
+ }
+
+ for (i = 0; i < MAIN_SEGS(sbi); i++) {
+ int blks = get_seg_entry(sbi, i)->valid_blocks;
+ int type = get_seg_entry(sbi, i)->type;
+
+ if (!blks)
+ continue;
+
+ if (blks == sbi->blocks_per_seg)
+ si->full_seg[type]++;
+ else
+ si->dirty_seg[type]++;
+ si->valid_blks[type] += blks;
+ }
+
for (i = 0; i < 2; i++) {
si->segment_count[i] = sbi->segment_count[i];
si->block_count[i] = sbi->block_count[i];
@@ -329,30 +349,50 @@ static int stat_show(struct seq_file *s, void *v)
seq_printf(s, "\nMain area: %d segs, %d secs %d zones\n",
si->main_area_segs, si->main_area_sections,
si->main_area_zones);
- seq_printf(s, " - COLD data: %d, %d, %d\n",
+ seq_printf(s, "TYPE %8s %8s %8s %10s %10s %10s\n",
+ "segno", "secno", "zoneno", "dirty_seg", "full_seg",
"valid_blk");
+ seq_printf(s, " - COLD data: %8d %8d %8d %10u %10u %10u\n",
si->curseg[CURSEG_COLD_DATA],
si->cursec[CURSEG_COLD_DATA],
- si->curzone[CURSEG_COLD_DATA]);
- seq_printf(s, " - WARM data: %d, %d, %d\n",
+ si->curzone[CURSEG_COLD_DATA],
+ si->dirty_seg[CURSEG_COLD_DATA],
+ si->full_seg[CURSEG_COLD_DATA],
+ si->valid_blks[CURSEG_COLD_DATA]);
+ seq_printf(s, " - WARM data: %8d %8d %8d %10u %10u %10u\n",
si->curseg[CURSEG_WARM_DATA],
si->cursec[CURSEG_WARM_DATA],
- si->curzone[CURSEG_WARM_DATA]);
- seq_printf(s, " - HOT data: %d, %d, %d\n",
+ si->curzone[CURSEG_WARM_DATA],
+ si->dirty_seg[CURSEG_WARM_DATA],
+ si->full_seg[CURSEG_WARM_DATA],
+ si->valid_blks[CURSEG_WARM_DATA]);
+ seq_printf(s, " - HOTdata: %8d %8d %8d %10u %10u %10u\n",
si->curseg[CURSEG_HOT_DATA],
si->cursec[CURSEG_HOT_DATA],
- si->curzone[CURSEG_HOT_DATA]);
- seq_printf(s, " - Dir dnode: %d, %d, %d\n",
+ si->curzone[CURSEG_HOT_DATA],
+ si->dirty_seg[CURSEG_HOT_DATA],
+ si->full_seg[CURSEG_HOT_DATA],
+ si->valid_blks[CURSEG_HOT_DATA]);
+ seq_printf(s, " - Dir dnode: %8d %8d %8d %10u %10u %10u\n",
si->curseg[CURSEG_HOT_NODE],
si->cursec[CURSEG_HOT_NODE],
- si->curzone[CURSEG_HOT_NODE]);
- seq_printf(s, " - File dnode: %d, %d, %d\n",
+ si->curzone[CURSEG_HOT_NODE],
+ si->dirty_seg[CURSEG_HOT_NODE],
+ si->full_seg[CURSEG_HOT_NODE],
+ si->valid_blks[CURSEG_HOT_NODE]);
+ seq_printf(s, " - File dnode: %8d %8d %8d %10u %10u %10u\n",
si->curseg[CURSEG_WARM_NODE],
si->cursec[CURSEG_WARM_NODE],
- si->curzone[CURSEG_WARM_NODE]);
- seq_printf(s, " - Indir nodes: %d, %d, %d\n",
+
[f2fs-dev] [PATCH v2 1/5] f2fs: fix to wait page writeback before update
Filesystem including f2fs should support stable page for special
device like software raid, however there is one missing path that
page could be updated while it is writeback state as below, fix
this.
- gc_node_segment
- f2fs_move_node_page
- __write_node_page
- set_page_writeback
- do_read_inode
- f2fs_init_extent_tree
- __f2fs_init_extent_tree
i_ext->len = 0;
Signed-off-by: Chao Yu
---
v2:
- add call path into message.
- remove unrelated changes.
fs/f2fs/extent_cache.c | 18 +-
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/inode.c| 3 +--
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index e60078460ad1..686c68b98610 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -325,9 +325,10 @@ static void __drop_largest_extent(struct extent_tree *et,
}
/* return true, if inode page is changed */
-static bool __f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent
*i_ext)
+static void __f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct f2fs_extent *i_ext = ipage ? _INODE(ipage)->i_ext : NULL;
struct extent_tree *et;
struct extent_node *en;
struct extent_info ei;
@@ -335,16 +336,18 @@ static bool __f2fs_init_extent_tree(struct inode *inode,
struct f2fs_extent *i_e
if (!f2fs_may_extent_tree(inode)) {
/* drop largest extent */
if (i_ext && i_ext->len) {
+ f2fs_wait_on_page_writeback(ipage, NODE, true, true);
i_ext->len = 0;
- return true;
+ set_page_dirty(ipage);
+ return;
}
- return false;
+ return;
}
et = __grab_extent_tree(inode);
if (!i_ext || !i_ext->len)
- return false;
+ return;
get_extent_info(, i_ext);
@@ -360,17 +363,14 @@ static bool __f2fs_init_extent_tree(struct inode *inode,
struct f2fs_extent *i_e
}
out:
write_unlock(>lock);
- return false;
}
-bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext)
+void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
{
- bool ret = __f2fs_init_extent_tree(inode, i_ext);
+ __f2fs_init_extent_tree(inode, ipage);
if (!F2FS_I(inode)->extent_tree)
set_inode_flag(inode, FI_NO_EXTENT);
-
- return ret;
}
static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cba9c0129f09..54cde692d88d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3821,7 +3821,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct
rb_root_cached *root,
bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
struct rb_root_cached *root);
unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int nr_shrink);
-bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext);
+void f2fs_init_extent_tree(struct inode *inode, struct page *ipage);
void f2fs_drop_extent_tree(struct inode *inode);
unsigned int f2fs_destroy_extent_node(struct inode *inode);
void f2fs_destroy_extent_tree(struct inode *inode);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 33affa788588..66969ae852b9 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -367,8 +367,7 @@ static int do_read_inode(struct inode *inode)
fi->i_pino = le32_to_cpu(ri->i_pino);
fi->i_dir_level = ri->i_dir_level;
- if (f2fs_init_extent_tree(inode, >i_ext))
- set_page_dirty(node_page);
+ f2fs_init_extent_tree(inode, node_page);
get_inline_info(inode, ri);
--
2.26.2
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove unnecessary judgments in __insert_free_nid()
On 2020/6/26 22:39, Liu Song via Linux-f2fs-devel wrote: > From: Liu Song > > The value of state must be equal to FREE_NID, so the if > condition judgment can be removed. > > Signed-off-by: Liu Song > --- > fs/f2fs/node.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index 03e24df1c84f..0adeb20f19c9 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -2118,8 +2118,7 @@ static int __insert_free_nid(struct f2fs_sb_info *sbi, > > f2fs_bug_on(sbi, state != i->state); > nm_i->nid_cnt[state]++; > - if (state == FREE_NID) > - list_add_tail(>list, _i->free_nid_list); > + list_add_tail(>list, _i->free_nid_list); In previous design, @state allow accepting both FREE_NID and PREALLOC_NID, If you remove that condition, it's not correct to add free nid entry into free_nid_list when passing PREALLOC_NID in @state, now, we only pass @state with FREE_NID, so it's better to remove that parameter directly. Thanks, > return 0; > } > > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix typo in comment of f2fs_do_add_link
On 2020/6/25 20:40, Liu Song via Linux-f2fs-devel wrote: > stakable/stackable > > Signed-off-by: Liu Song Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: avoid readahead race condition
On 2020/6/24 9:21, Jaegeuk Kim wrote:
> If two readahead threads having same offset enter in readpages, every read
> IOs are split and issued to the disk which giving lower bandwidth.
>
> This patch tries to avoid redundant readahead calls.
>
> Signed-off-by: Jaegeuk Kim
> ---
> fs/f2fs/data.c | 15 +++
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/super.c | 2 ++
> 3 files changed, 18 insertions(+)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index dfd3225153570..1886d83bc5f15 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2292,6 +2292,7 @@ static int f2fs_mpage_readpages(struct inode *inode,
> unsigned nr_pages = rac ? readahead_count(rac) : 1;
> unsigned max_nr_pages = nr_pages;
> int ret = 0;
> + bool drop_ra = false;
>
> map.m_pblk = 0;
> map.m_lblk = 0;
> @@ -2302,6 +2303,17 @@ static int f2fs_mpage_readpages(struct inode *inode,
> map.m_seg_type = NO_CHECK_TYPE;
> map.m_may_create = false;
>
> + /*
> + * Two readahead threads for same address range can cause race condition
> + * which fragments sequential read IOs. So let's avoid each other.
> + */
> + if (rac && readahead_count(rac)) {
> + if (F2FS_I(inode)->ra_offset == readahead_index(rac))
> + drop_ra = true;
I guess you missed to return at somewhere when drop_ra is true?
thanks,
> + else
> + F2FS_I(inode)->ra_offset = readahead_index(rac);
> + }
> +
> for (; nr_pages; nr_pages--) {
> if (rac) {
> page = readahead_page(rac);
> @@ -2368,6 +2380,9 @@ static int f2fs_mpage_readpages(struct inode *inode,
> }
> if (bio)
> __submit_bio(F2FS_I_SB(inode), bio, DATA);
> +
> + if (rac && readahead_count(rac) && !drop_ra)
> + F2FS_I(inode)->ra_offset = -1;
> return ret;
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 7fb2a1a334388..753782426feac 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -809,6 +809,7 @@ struct f2fs_inode_info {
> struct list_head inmem_pages; /* inmemory pages managed by f2fs */
> struct task_struct *inmem_task; /* store inmemory task */
> struct mutex inmem_lock;/* lock for inmemory pages */
> + pgoff_t ra_offset; /* ongoing readahead offset */
> struct extent_tree *extent_tree;/* cached extent_tree entry */
>
> /* avoid racing between foreground op and gc */
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 7326522057378..80cb7cd358f84 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1015,6 +1015,8 @@ static struct inode *f2fs_alloc_inode(struct
> super_block *sb)
> /* Will be used by directory only */
> fi->i_dir_level = F2FS_SB(sb)->dir_level;
>
> + fi->ra_offset = -1;
> +
> return >vfs_inode;
> }
>
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/5] f2fs: show more debug info for per-temperature log
On 2020/6/25 6:31, Jaegeuk Kim wrote:
> On 06/18, Chao Yu wrote:
>> - Add to account and show per-log dirty_seg, full_seg and valid_blocks
>> in debugfs.
>> - reformat printed info.
>>
>> TYPEsegnosecno zoneno dirty_seg full_seg valid_blk
>> - COLD data: 1523 1523 1523 1 0399
>> - WARM data: 769 769 769 20255 133098
>> - HOTdata: 767 767 767 9 0167
>> - Dir dnode: 22 22 22 3 0 70
>> - File dnode: 722 722 722 14 10 6505
>> - Indir nodes:222 1 0 3
>>
>> Signed-off-by: Chao Yu
>> ---
>> fs/f2fs/debug.c | 67 -
>> fs/f2fs/f2fs.h | 3 +++
>> 2 files changed, 58 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>> index 0dbcb0f9c019..aa1fd2de11ba 100644
>> --- a/fs/f2fs/debug.c
>> +++ b/fs/f2fs/debug.c
>> @@ -174,6 +174,29 @@ static void update_general_status(struct f2fs_sb_info
>> *sbi)
>> for (i = META_CP; i < META_MAX; i++)
>> si->meta_count[i] = atomic_read(>meta_count[i]);
>>
>> +for (i = 0; i < NO_CHECK_TYPE; i++) {
>> +si->dirty_seg[i] = 0;
>> +si->full_seg[i] = 0;
>> +si->valid_blks[i] = 0;
>> +}
>> +
>> +for (i = 0; i < MAIN_SEGS(sbi); i++) {
>> +int blks = get_seg_entry(sbi, i)->valid_blocks;
>> +int type = get_seg_entry(sbi, i)->type;
>> +
>> +if (!blks)
>> +continue;
>> +
>> +if (IS_CURSEG(sbi, i))
>> +continue;
>
> How about adding current segments as well? Especially, it's hard to see any
> valid blocks for cold node with this.
Better, let me update this patch.
>
>> +
>> +if (blks == sbi->blocks_per_seg)
>> +si->full_seg[type]++;
>> +else
>> +si->dirty_seg[type]++;
>> +si->valid_blks[type] += blks;
>> +}
>> +
>> for (i = 0; i < 2; i++) {
>> si->segment_count[i] = sbi->segment_count[i];
>> si->block_count[i] = sbi->block_count[i];
>> @@ -329,30 +352,50 @@ static int stat_show(struct seq_file *s, void *v)
>> seq_printf(s, "\nMain area: %d segs, %d secs %d zones\n",
>> si->main_area_segs, si->main_area_sections,
>> si->main_area_zones);
>> -seq_printf(s, " - COLD data: %d, %d, %d\n",
>> +seq_printf(s, "TYPE %8s %8s %8s %10s %10s %10s\n",
>> + "segno", "secno", "zoneno", "dirty_seg", "full_seg",
>> "valid_blk");
>> +seq_printf(s, " - COLD data: %8d %8d %8d %10u %10u %10u\n",
>> si->curseg[CURSEG_COLD_DATA],
>> si->cursec[CURSEG_COLD_DATA],
>> - si->curzone[CURSEG_COLD_DATA]);
>> -seq_printf(s, " - WARM data: %d, %d, %d\n",
>> + si->curzone[CURSEG_COLD_DATA],
>> + si->dirty_seg[CURSEG_COLD_DATA],
>> + si->full_seg[CURSEG_COLD_DATA],
>> + si->valid_blks[CURSEG_COLD_DATA]);
>> +seq_printf(s, " - WARM data: %8d %8d %8d %10u %10u %10u\n",
>> si->curseg[CURSEG_WARM_DATA],
>> si->cursec[CURSEG_WARM_DATA],
>> - si->curzone[CURSEG_WARM_DATA]);
>> -seq_printf(s, " - HOT data: %d, %d, %d\n",
>> + si->curzone[CURSEG_WARM_DATA],
>> + si->dirty_seg[CURSEG_WARM_DATA],
>> + si->full_seg[CURSEG_WARM_DATA],
>> + si->valid_blks[CURSEG_WARM_DATA]);
>> +seq_printf(s, " - HOTdata: %8d %8d %8d %10u %10u %10u\n",
>> si->curseg[CURSEG_HOT_DATA],
>> si->cursec[CURSEG_HOT_DATA],
>> - si->curzone[CURSEG_HOT_DATA]);
>> -seq_printf(s, " - Dir dnode: %d, %d, %d\n",
>> + si->curzone[CURSEG_HOT_DATA],
>> + si->dirty_seg[CURSEG_HOT_DATA],
>> + si->full_seg[CURSEG_HOT_DATA],
>> + si->valid_blks[CURSEG_HOT_DATA]);
>> +seq_printf(s, " - Dir dnode: %8d %8d %8d %10u %10u %10u\n",
>> si->curseg[CURSEG_HOT_NODE],
>> si->cursec[CURSEG_HOT_NODE],
>> - si->curzone[CURSEG_HOT_NODE]);
>> -seq_printf(s, " - File dnode: %d, %d, %d\n",
>> + si->curzone[CURSEG_HOT_NODE],
>> + si->dirty_seg[CURSEG_HOT_NODE],
>> + si->full_seg[CURSEG_HOT_NODE],
>> +
Re: [f2fs-dev] [PATCH v3] f2fs: add f2fs_gc exception handle in f2fs_ioc_gc_range
On 2020/6/24 17:40, Qilong Zhang wrote:
> When f2fs_ioc_gc_range performs multiple segments gc ops, the return
> value of f2fs_ioc_gc_range is determined by the last segment gc ops.
> If its ops failed, the f2fs_ioc_gc_range will be considered to be failed
> despite some of previous segments gc ops succeeded. Therefore, so we
> fix: Redefine the return value of getting victim ops and add exception
> handle for f2fs_gc. In particular, 1).if target has no valid block, it
> will go on. 2).if target sectoion has valid block(s), but it is current
> section, we will reminder the caller.
>
> Signed-off-by: Qilong Zhang
> ---
> fs/f2fs/file.c| 5 +
> fs/f2fs/gc.c | 19 +--
> fs/f2fs/segment.c | 4 ++--
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 3268f8dd59bb..209dd9cb4b7b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2527,6 +2527,11 @@ static int f2fs_ioc_gc_range(struct file *filp,
> unsigned long arg)
> }
>
> ret = f2fs_gc(sbi, range.sync, true, GET_SEGNO(sbi, range.start));
> + if (ret) {
> + if (ret == -EAGAIN)
> + ret = -EBUSY;
> + goto out;
> + }
> range.start += BLKS_PER_SEC(sbi);
> if (range.start <= end)
> goto do_more;
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 5b95d5a146eb..297e53ef4cb1 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -321,6 +321,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
> unsigned int secno, last_victim;
> unsigned int last_segment;
> unsigned int nsearched = 0;
> + int ret;
>
> mutex_lock(_i->seglist_lock);
> last_segment = MAIN_SECS(sbi) * sbi->segs_per_sec;
> @@ -332,12 +333,18 @@ static int get_victim_by_default(struct f2fs_sb_info
> *sbi,
> p.min_cost = get_max_cost(sbi, );
>
> if (*result != NULL_SEGNO) {
> - if (get_valid_blocks(sbi, *result, false) &&
> - !sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result)))
> + ret = 0;
> + if (!get_valid_blocks(sbi, *result, false))
> + goto out;
> +
> + if (sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result)))
> + ret = -EAGAIN;
Look at this again, it looks EBUSY will be more precise here? how about changing
to return EBUSY?
#define EBUSY 16 /* Device or resource busy */
#define EAGAIN 11 /* Try again */
Otherwise it looks good to me, since it's nitpick, so feel free to add:
Reviewed-by: Chao Yu
Thanks,
> + else
> p.min_segno = *result;
> goto out;
> }
>
> + ret = -ENODATA;
> if (p.max_search == 0)
> goto out;
>
> @@ -440,6 +447,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
> else
> set_bit(secno, dirty_i->victim_secmap);
> }
> + ret = 0;
>
> }
> out:
> @@ -449,7 +457,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
> prefree_segments(sbi), free_segments(sbi));
> mutex_unlock(_i->seglist_lock);
>
> - return (p.min_segno == NULL_SEGNO) ? 0 : 1;
> + return ret;
> }
>
> static const struct victim_selection default_v_ops = {
> @@ -1333,10 +1341,9 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
> ret = -EINVAL;
> goto stop;
> }
> - if (!__get_victim(sbi, , gc_type)) {
> - ret = -ENODATA;
> + ret = __get_victim(sbi, , gc_type);
> + if (ret)
> goto stop;
> - }
>
> seg_freed = do_garbage_collect(sbi, segno, _list, gc_type);
> if (gc_type == FG_GC && seg_freed == sbi->segs_per_sec)
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 196f31503511..b9fd93761b0a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2605,7 +2605,7 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi,
> int type)
> bool reversed = false;
>
> /* f2fs_need_SSR() already forces to do this */
> - if (v_ops->get_victim(sbi, , BG_GC, type, SSR)) {
> + if (!v_ops->get_victim(sbi, , BG_GC, type, SSR)) {
> curseg->next_segno = segno;
> return 1;
> }
> @@ -2632,7 +2632,7 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi,
> int type)
> for (; cnt-- > 0; reversed ? i-- : i++) {
> if (i == type)
> continue;
> - if (v_ops->get_victim(sbi, , BG_GC, i, SSR)) {
> + if (!v_ops->get_victim(sbi, , BG_GC, i, SSR)) {
> curseg->next_segno = segno;
> return 1;
> }
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
Re: [f2fs-dev] [PATCH 1/5] f2fs: fix to wait page writeback before update
On 2020/6/24 23:55, Jaegeuk Kim wrote:
> On 06/22, Chao Yu wrote:
>> On 2020/6/22 0:38, Jaegeuk Kim wrote:
>>> On 06/20, Chao Yu wrote:
On 2020/6/20 6:47, Jaegeuk Kim wrote:
> On 06/19, Chao Yu wrote:
>> On 2020/6/19 13:49, Jaegeuk Kim wrote:
>>> On 06/19, Chao Yu wrote:
Hi Jaegeuk,
On 2020/6/19 7:59, Jaegeuk Kim wrote:
> Hi Chao,
>
> On 06/18, Chao Yu wrote:
>> to make page content stable for special device like raid.
>
> Could you elaborate the problem a bit?
Some devices like raid5 wants page content to be stable, because
it will calculate parity info based page content, if page is not
stable, parity info could be corrupted, result in data inconsistency
in stripe.
>>>
>>> I don't get the point, since those pages are brand new pages which were
>>> not
>>> modified before. If it's on writeback, we should not modify them
>>> regardless
>>> of whatever raid configuration. For example, f2fs_new_node_page() waits
>>> for
>>> writeback. Am I missing something?
>>
>> I think we should use f2fs_bug_on(, PageWriteback()) rather than
>> f2fs_wait_on_page_writeback() for brand new page which is allocated just
>> now.
>> For other paths, we can keep rule that waiting for writeback before
>> updating.
>>
>> How do you think?
>>
>> Thanks,
>>
>>>
Thanks,
>
>>
>> Signed-off-by: Chao Yu
>> ---
>> fs/f2fs/dir.c | 2 ++
>> fs/f2fs/extent_cache.c | 18 +-
>> fs/f2fs/f2fs.h | 2 +-
>> fs/f2fs/file.c | 1 +
>> fs/f2fs/inline.c | 2 ++
>> fs/f2fs/inode.c| 3 +--
>> 6 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index d35976785e8c..91e86747a604 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -495,6 +495,8 @@ static int make_empty_dir(struct inode *inode,
>> if (IS_ERR(dentry_page))
>> return PTR_ERR(dentry_page);
>>
>> +f2fs_bug_on(F2FS_I_SB(inode), PageWriteback(dentry_page));
>> +
>> dentry_blk = page_address(dentry_page);
>>
>> make_dentry_ptr_block(NULL, , dentry_blk);
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index e60078460ad1..686c68b98610 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -325,9 +325,10 @@ static void __drop_largest_extent(struct
>> extent_tree *et,
>> }
>>
>> /* return true, if inode page is changed */
>> -static bool __f2fs_init_extent_tree(struct inode *inode, struct
>> f2fs_extent *i_ext)
>> +static void __f2fs_init_extent_tree(struct inode *inode, struct
>> page *ipage)
>> {
>> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> +struct f2fs_extent *i_ext = ipage ? _INODE(ipage)->i_ext :
>> NULL;
>> struct extent_tree *et;
>> struct extent_node *en;
>> struct extent_info ei;
>> @@ -335,16 +336,18 @@ static bool __f2fs_init_extent_tree(struct
>> inode *inode, struct f2fs_extent *i_e
>> if (!f2fs_may_extent_tree(inode)) {
>> /* drop largest extent */
>> if (i_ext && i_ext->len) {
>> +f2fs_wait_on_page_writeback(ipage, NODE, true,
>> true);
>> i_ext->len = 0;
>> -return true;
>> +set_page_dirty(ipage);
>> +return;
>> }
>> -return false;
>> +return;
>> }
>>
>> et = __grab_extent_tree(inode);
>>
>> if (!i_ext || !i_ext->len)
>> -return false;
>> +return;
>>
>> get_extent_info(, i_ext);
>>
>> @@ -360,17 +363,14 @@ static bool __f2fs_init_extent_tree(struct
>> inode *inode, struct f2fs_extent *i_e
>> }
>> out:
>> write_unlock(>lock);
>> -return false;
>> }
>>
>> -bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent
>> *i_ext)
>> +void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
>> {
>> -bool ret = __f2fs_init_extent_tree(inode, i_ext);
>> +__f2fs_init_extent_tree(inode, ipage);
>>
>> if (!F2FS_I(inode)->extent_tree)
>> set_inode_flag(inode,
[f2fs-dev] [PATCH 2/4] media: atomisp: Replace trace_printk by pr_info
trace_printk should not be used in production code, replace it
call with pr_info.
Signed-off-by: Nicolas Boichat
---
drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c
b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 42fef17798622f1..2bd39b4939f16d2 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -735,11 +735,11 @@ ia_css_ptr hmm_host_vaddr_to_hrt_vaddr(const void *ptr)
void hmm_show_mem_stat(const char *func, const int line)
{
- trace_printk("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
-hmm_mem_stat.tol_cnt,
-hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
-hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
-hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
+ pr_info("tol_cnt=%d usr_size=%d res_size=%d res_cnt=%d sys_size=%d
dyc_thr=%d dyc_size=%d.\n",
+ hmm_mem_stat.tol_cnt,
+ hmm_mem_stat.usr_size, hmm_mem_stat.res_size,
+ hmm_mem_stat.res_cnt, hmm_mem_stat.sys_size,
+ hmm_mem_stat.dyc_thr, hmm_mem_stat.dyc_size);
}
void hmm_init_mem_stat(int res_pgnr, int dyc_en, int dyc_pgnr)
--
2.27.0.212.ge8ba1cc988-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v2 4/4] kernel/trace: Add TRACING_ALLOW_PRINTK config option
trace_printk is meant as a debugging tool, and should not be
compiled into production code without specific debug Kconfig
options enabled, or source code changes, as indicated by the
warning that shows up on boot if any trace_printk is called:
** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
** **
** trace_printk() being used. Allocating extra memory. **
** **
** This means that this is a DEBUG kernel and it is **
** unsafe for production use. **
If this option is set to n, the kernel will generate a
build-time error if trace_printk is used.
Note that the code to handle trace_printk is still present,
so this does not prevent people from compiling out-of-tree
kernel modules with the option set to =y, or BPF programs.
Signed-off-by: Nicolas Boichat
---
Changes since v1:
- Use static_assert instead of __static_assert (Jason Gunthorpe)
- Fix issues that can be detected by this patch (running some
randconfig in a loop, kernel test robot, or manual inspection),
by:
- Making some debug config options that use trace_printk depend
on the new config option.
- Adding 3 patches before this one.
There is a question from Alexei whether the warning is warranted,
and it's possibly too strongly worded, but the fact is, we do
not want trace_printk to be sprinkled in kernel code by default,
unless a very specific Kconfig command is enabled (or preprocessor
macro).
There's at least 3 reasons that I can come up with:
1. trace_printk introduces some overhead.
2. If the kernel keeps adding always-enabled trace_printk, it will
be much harder for developers to make use of trace_printk for
debugging.
3. People may assume that trace_printk is for debugging only, and
may accidentally output sensitive data (theoritical at this
stage).
drivers/gpu/drm/i915/Kconfig.debug | 4 ++--
fs/f2fs/Kconfig| 1 +
include/linux/kernel.h | 17 -
kernel/trace/Kconfig | 10 ++
samples/Kconfig| 2 ++
5 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig.debug
b/drivers/gpu/drm/i915/Kconfig.debug
index 1cb28c20807c59d..fa30f9bdc82311f 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -84,7 +84,7 @@ config DRM_I915_ERRLOG_GEM
config DRM_I915_TRACE_GEM
bool "Insert extra ftrace output from the GEM internals"
depends on DRM_I915_DEBUG_GEM
- select TRACING
+ depends on TRACING_ALLOW_PRINTK
default n
help
Enable additional and verbose debugging output that will spam
@@ -98,7 +98,7 @@ config DRM_I915_TRACE_GEM
config DRM_I915_TRACE_GTT
bool "Insert extra ftrace output from the GTT internals"
depends on DRM_I915_DEBUG_GEM
- select TRACING
+ depends on TRACING_ALLOW_PRINTK
default n
help
Enable additional and verbose debugging output that will spam
diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index d13c5c6a978769b..d161d96cc1b7418 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -80,6 +80,7 @@ config F2FS_IO_TRACE
bool "F2FS IO tracer"
depends on F2FS_FS
depends on FUNCTION_TRACER
+ depends on TRACING_ALLOW_PRINTK
help
F2FS IO trace is based on a function trace, which gathers process
information and block IO patterns in the filesystem level.
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 196607aaf653082..8abce95b0c95a0e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -721,10 +721,15 @@ do {
\
#define trace_printk(fmt, ...) \
do { \
char ___STR[] = __stringify((__VA_ARGS__)); \
+ \
+ static_assert( \
+ IS_ENABLED(CONFIG_TRACING_ALLOW_PRINTK),\
+ "trace_printk called, please enable
CONFIG_TRACING_ALLOW_PRINTK."); \
+ \
if (sizeof(___STR) > 3) \
do_trace_printk(fmt, ##__VA_ARGS__);\
else\
- trace_puts(fmt);\
+ do_trace_puts(fmt); \
} while (0)
#define do_trace_printk(fmt, args...) \
@@ -773,6 +778,13 @@ int __trace_printk(unsigned long ip, const char *fmt, ...);
*/
#define trace_puts(str) ({ \
+ static_assert( \
+
[f2fs-dev] [PATCH 3/4] media: camss: vfe: Use trace_printk for debugging only
trace_printk should not be used in production code. Since
tracing interrupts is presumably latency sensitive, pr_dbg is
not appropriate, so guard the call with a preprocessor symbol
that can be defined for debugging purpose.
Signed-off-by: Nicolas Boichat
---
drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++
drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
index 174a36be6f5d866..0c57171fae4f9e9 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c
@@ -936,8 +936,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
vfe->ops->isr_read(vfe, , );
+#ifdef CAMSS_VFE_TRACE_IRQ
trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
value0, value1);
+#endif
if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
vfe->isr_ops.reset_ack(vfe);
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
index 0dca8bf9281e774..307675925e5c779 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c
@@ -1058,8 +1058,10 @@ static irqreturn_t vfe_isr(int irq, void *dev)
vfe->ops->isr_read(vfe, , );
+#ifdef CAMSS_VFE_TRACE_IRQ
trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n",
value0, value1);
+#endif
if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK)
vfe->isr_ops.reset_ack(vfe);
--
2.27.0.212.ge8ba1cc988-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
trace_printk should not be used in production code, replace it
call with dev_dbg.
Signed-off-by: Nicolas Boichat
---
Unclear why a trace_printk was used in the first place, it's
possible that some rate-limiting is necessary here.
drivers/usb/cdns3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
*priv_dev,
if ((priv_req->flags & REQUEST_INTERNAL) ||
(priv_ep->flags & EP_TDLCHK_EN) ||
priv_ep->use_streams) {
- trace_printk("Blocking external request\n");
+ dev_dbg(priv_dev->dev, "Blocking external request\n");
return ret;
}
}
--
2.27.0.212.ge8ba1cc988-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 0/4] Detect and remove trace_printk
trace_printk is meant as a debugging tool, and should not be compiled into production code without specific debug Kconfig options enabled or source code changes. Patches 1 to 3 remove/disable trace_printk that should not be enabled by default. Patch 4 adds a config option that can be used to detect such trace_printk at compile time (instead of printing a nasty warning at runtime). Nicolas Boichat (4): usb: cdns3: gadget: Replace trace_printk by dev_dbg media: atomisp: Replace trace_printk by pr_info media: camss: vfe: Use trace_printk for debugging only kernel/trace: Add TRACING_ALLOW_PRINTK config option drivers/gpu/drm/i915/Kconfig.debug | 4 ++-- .../media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ .../media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ drivers/staging/media/atomisp/pci/hmm/hmm.c | 10 +- drivers/usb/cdns3/gadget.c | 2 +- fs/f2fs/Kconfig | 1 + include/linux/kernel.h | 17 - kernel/trace/Kconfig| 10 ++ samples/Kconfig | 2 ++ 9 files changed, 41 insertions(+), 9 deletions(-) -- 2.27.0.212.ge8ba1cc988-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
