linux-kernel - Re: [PATCH] f2fs-tools: set cold flag for non-dir node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4a2d19c4-1ede-0172-fe50-c8464ee341df@huawei.com>
Date:   Fri, 19 Jun 2020 09:31:22 +0800
From:   Chao Yu <yuchao0@...wei.com>
To:     <zhaowuyun@...gtech.com>, <jaegeuk@...nel.org>
CC:     <linux-f2fs-devel@...ts.sourceforge.net>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] f2fs-tools: set cold flag for non-dir node

On 2020/6/18 20:48, zhaowuyun@...gtech.com wrote:
> From: Wuyun Zhao <zhaowuyun@...gtech.com>

Thanks for the patch. :)

Please add commit message here.

> 
> Signed-off-by: Wuyun Zhao <zhaowuyun@...gtech.com>
> ---
>  fsck/dir.c  |  1 +
>  fsck/node.c |  1 +
>  fsck/node.h | 11 +++++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/fsck/dir.c b/fsck/dir.c
> index 5f4f75e..dc03c98 100644
> --- a/fsck/dir.c
> +++ b/fsck/dir.c
> @@ -522,6 +522,7 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
>  	node_blk->footer.nid = cpu_to_le32(de->ino);
>  	node_blk->footer.flag = 0;
>  	node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> +	set_cold_node(node_blk, S_ISDIR(mode));
>  
>  	if (S_ISDIR(mode)) {
>  		make_empty_dir(sbi, node_blk);
> diff --git a/fsck/node.c b/fsck/node.c
> index 229a99c..1d291ca 100644
> --- a/fsck/node.c
> +++ b/fsck/node.c
> @@ -79,6 +79,7 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
>  	node_blk->footer.ino = f2fs_inode->footer.ino;
>  	node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
>  	node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> +	set_cold_node(node_blk, S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));

How about wrapping these node footer fields assignment into a function? then
we can reuse this in other places.

void set_node_footer(nid, ino, ofs, ver, is_dir)
{
	node_blk->footer.nid = cpu_to_le32(nid);
	node_blk->footer.ino = f2fs_inode->footer.ino;
	node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
	node_blk->footer.cp_ver = ckpt->checkpoint_ver;
	set_cold_node(node_blk, S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
}

>  
>  	type = CURSEG_COLD_NODE;
>  	if (IS_DNODE(node_blk)) {
> diff --git a/fsck/node.h b/fsck/node.h
> index 6bce1fb..99139b1 100644
> --- a/fsck/node.h
> +++ b/fsck/node.h
> @@ -161,6 +161,17 @@ static inline int is_node(struct f2fs_node *node_blk, int type)
>  	return le32_to_cpu(node_blk->footer.flag) & (1 << type);
>  }

Beside this, I think we need to use set_node_footer() in:
- f2fs_write_root_inode
- f2fs_write_qf_inode
- f2fs_write_lpf_inode

as well to fix mkfs bugs.

Thanks,

>  
> +static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
> +{
> +	unsigned int flag = le32_to_cpu(rn->footer.flag);
> +
> +	if (is_dir)
> +		flag &= ~(0x1 << COLD_BIT_SHIFT);
> +	else
> +		flag |= (0x1 << COLD_BIT_SHIFT);
> +	rn->footer.flag = cpu_to_le32(flag);
> +}
> +
>  #define is_fsync_dnode(node_blk)	is_node(node_blk, FSYNC_BIT_SHIFT)
>  #define is_dent_dnode(node_blk)		is_node(node_blk, DENT_BIT_SHIFT)
>  
>