| 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
| ||
|
Date: Tue, 6 Jan 2015 11:44:19 -0800
From: Jaegeuk Kim <jaegeuk@...nel.org>
To: Chao Yu <chao2.yu@...sung.com>
Cc: Changman Lee <cm224.lee@...sung.com>,
linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org
Subject: Re: [f2fs-dev][PATCH 2/2] f2fs: enable recover_xattr_data to avoid
cp when fsync after operating xattr
Hi Chao,
On Tue, Jan 06, 2015 at 02:29:40PM +0800, Chao Yu wrote:
> Now if we call fsync() after we update the xattr date belongs to the file, f2fs
> will do checkpoint to keep data.
> This can cause low performance because checkpoint block most operation and write
> lots of blocks. So we'd better to avoid doing checkpoint by writing modified
> xattr node page to warm node segment, and then it can be recovered when we mount
> this device later on.
You're trying to change the writing policy as xattr blocks are written into
WARM_NODE area instead of COLD_NODE area.
I don't think xattrs are frequently changed between each fsync calls.
How do you think?
Thanks,
>
> Signed-off-by: Chao Yu <chao2.yu@...sung.com>
> ---
> fs/f2fs/f2fs.h | 3 +--
> fs/f2fs/file.c | 3 ---
> fs/f2fs/node.c | 28 ++++++++++++++++------------
> fs/f2fs/node.h | 2 +-
> fs/f2fs/recovery.c | 4 +++-
> fs/f2fs/xattr.c | 3 ---
> 6 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c48847e..dfbdd64 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -301,7 +301,6 @@ struct f2fs_inode_info {
> f2fs_hash_t chash; /* hash value of given file name */
> unsigned int clevel; /* maximum level of given file name */
> nid_t i_xattr_nid; /* node id that contains xattrs */
> - unsigned long long xattr_ver; /* cp version of xattr modification */
> struct extent_info ext; /* in-memory extent cache entry */
> struct inode_entry *dirty_dir; /* the pointer of dirty dir */
>
> @@ -1391,7 +1390,7 @@ bool alloc_nid(struct f2fs_sb_info *, nid_t *);
> void alloc_nid_done(struct f2fs_sb_info *, nid_t);
> void alloc_nid_failed(struct f2fs_sb_info *, nid_t);
> void recover_inline_xattr(struct inode *, struct page *);
> -void recover_xattr_data(struct inode *, struct page *, block_t);
> +int recover_xattr_data(struct inode *, struct page *, block_t);
> int recover_inode_page(struct f2fs_sb_info *, struct page *);
> int restore_node_summary(struct f2fs_sb_info *, unsigned int,
> struct f2fs_summary_block *);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f172ddc4..fb26a43 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -129,8 +129,6 @@ static inline bool need_do_checkpoint(struct inode *inode)
> need_cp = true;
> else if (!is_checkpointed_node(sbi, F2FS_I(inode)->i_pino))
> need_cp = true;
> - else if (F2FS_I(inode)->xattr_ver == cur_cp_version(F2FS_CKPT(sbi)))
> - need_cp = true;
> else if (test_opt(sbi, FASTBOOT))
> need_cp = true;
> else if (sbi->active_logs == 2)
> @@ -156,7 +154,6 @@ static void try_to_fix_pino(struct inode *inode)
> nid_t pino;
>
> down_write(&fi->i_sem);
> - fi->xattr_ver = 0;
> if (file_wrong_pino(inode) && inode->i_nlink == 1 &&
> get_parent_ino(inode, &pino)) {
> fi->i_pino = pino;
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index a7cb0db..675fa65 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -863,9 +863,6 @@ int truncate_xattr_node(struct inode *inode, struct page *page)
>
> F2FS_I(inode)->i_xattr_nid = 0;
>
> - /* need to do checkpoint during fsync */
> - F2FS_I(inode)->xattr_ver = cur_cp_version(F2FS_CKPT(sbi));
> -
> set_new_dnode(&dn, inode, page, npage, nid);
>
> if (page)
> @@ -1655,12 +1652,13 @@ update_inode:
> f2fs_put_page(ipage, 1);
> }
>
> -void recover_xattr_data(struct inode *inode, struct page *page, block_t blkaddr)
> +int recover_xattr_data(struct inode *inode, struct page *page, block_t blkaddr)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> nid_t prev_xnid = F2FS_I(inode)->i_xattr_nid;
> nid_t new_xnid = nid_of_node(page);
> struct node_info ni;
> + struct page *xpage;
>
> /* 1: invalidate the previous xattr nid */
> if (!prev_xnid)
> @@ -1674,21 +1672,27 @@ void recover_xattr_data(struct inode *inode, struct page *page, block_t blkaddr)
> set_node_addr(sbi, &ni, NULL_ADDR, false);
>
> recover_xnid:
> - /* 2: allocate new xattr nid */
> + /* 2: update xattr nid in inode */
> + remove_free_nid(NM_I(sbi), new_xnid);
> + F2FS_I(inode)->i_xattr_nid = new_xnid;
> if (unlikely(!inc_valid_node_count(sbi, inode)))
> f2fs_bug_on(sbi, 1);
> + update_inode_page(inode);
> +
> + /* 3: update and make xattr node page dirty */
> + xpage = grab_cache_page(NODE_MAPPING(sbi), new_xnid);
> + if (!xpage)
> + return -ENOMEM;
> +
> + memcpy(F2FS_NODE(xpage), F2FS_NODE(page), PAGE_CACHE_SIZE);
>
> - remove_free_nid(NM_I(sbi), new_xnid);
> get_node_info(sbi, new_xnid, &ni);
> ni.ino = inode->i_ino;
> set_node_addr(sbi, &ni, NEW_ADDR, false);
> - F2FS_I(inode)->i_xattr_nid = new_xnid;
> + set_page_dirty(xpage);
> + f2fs_put_page(xpage, 1);
>
> - /* 3: update xattr blkaddr */
> - refresh_sit_entry(sbi, NEW_ADDR, blkaddr);
> - set_node_addr(sbi, &ni, blkaddr, false);
> -
> - update_inode_page(inode);
> + return 0;
> }
>
> int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
> index cac8a3d..55eb1e7 100644
> --- a/fs/f2fs/node.h
> +++ b/fs/f2fs/node.h
> @@ -300,7 +300,7 @@ static inline bool IS_DNODE(struct page *node_page)
> unsigned int ofs = ofs_of_node(node_page);
>
> if (f2fs_has_xattr_block(ofs))
> - return false;
> + return true;
>
> if (ofs == 3 || ofs == 4 + NIDS_PER_BLOCK ||
> ofs == 5 + 2 * NIDS_PER_BLOCK)
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 9160a37..4f2c197 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -346,7 +346,9 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
> if (IS_INODE(page)) {
> recover_inline_xattr(inode, page);
> } else if (f2fs_has_xattr_block(ofs_of_node(page))) {
> - recover_xattr_data(inode, page, blkaddr);
> + err = recover_xattr_data(inode, page, blkaddr);
> + if (!err)
> + recovered++;
> goto out;
> }
>
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index 5072bf9..3d13ca2 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -391,9 +391,6 @@ static inline int write_all_xattrs(struct inode *inode, __u32 hsize,
> sizeof(struct node_footer));
> set_page_dirty(xpage);
> f2fs_put_page(xpage, 1);
> -
> - /* need to checkpoint during fsync */
> - F2FS_I(inode)->xattr_ver = cur_cp_version(F2FS_CKPT(sbi));
> return 0;
> }
>
> --
> 2.2.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists