Hi Wanpeng,
> -----Original Message-----
> From: Wanpeng Li [mailto:[email protected]]
> Sent: Wednesday, March 11, 2015 7:52 PM
> To: Jaegeuk Kim
> Cc: Changman Lee; Chao Yu; [email protected];
> [email protected]; [email protected]; Wanpeng Li
> Subject: [PATCH RFC] f2fs: add fast symlink
>
> This patch add fast symlink for f2fs, I'm not sure if inline
> interference with fast symlink, f2fs_follow_link can't be called
> in my patch, so my patch still can't work correctly, please help
> review and give comments to help me out.
>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> fs/f2fs/f2fs.h | 9 +++++++++
> fs/f2fs/file.c | 2 ++
> fs/f2fs/inode.c | 15 +++++++++++++--
> fs/f2fs/namei.c | 44 +++++++++++++++++++++++++++++++++++++++++---
> 4 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 511d6cd..5938318 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1370,6 +1370,14 @@ static inline void f2fs_stop_checkpoint(struct
> f2fs_sb_info *sbi)
> sbi->sb->s_flags |= MS_RDONLY;
> }
>
> +/*
> + * Test whether an inode is a fast symlink.
> + */
> +static inline int f2fs_inode_is_fast_symlink(struct inode *inode)
> +{
> + return (S_ISLNK(inode->i_mode) && inode->i_blocks == 0);
How about introducing FI_FAST_SYMLINK & F2FS_FAST_SYMLINK to indicate whether
the inode is fast symlink or not?
So here could be:
return (S_ISLNK(inode->i_mode) &&
is_inode_flag_set(F2FS_I(inode), FI_FAST_SYMLINK));
> +}
> +
> #define get_inode_mode(i) \
> ((is_inode_flag_set(F2FS_I(i), FI_ACL_MODE)) ? \
> (F2FS_I(i)->i_acl_mode) : ((i)->i_mode))
> @@ -1746,6 +1754,7 @@ extern const struct address_space_operations
> f2fs_node_aops;
> extern const struct address_space_operations f2fs_meta_aops;
> extern const struct inode_operations f2fs_dir_inode_operations;
> extern const struct inode_operations f2fs_symlink_inode_operations;
> +extern const struct inode_operations f2fs_fast_symlink_inode_operations;
> extern const struct inode_operations f2fs_special_inode_operations;
> extern struct kmem_cache *inode_entry_slab;
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f1341c7..05a3d4f 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -546,6 +546,8 @@ void f2fs_truncate(struct inode *inode)
> if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> S_ISLNK(inode->i_mode)))
> return;
> + if (f2fs_inode_is_fast_symlink(inode))
> + return;
>
> trace_f2fs_truncate(inode);
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index b508744..627fe6b 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -13,6 +13,7 @@
> #include <linux/buffer_head.h>
> #include <linux/writeback.h>
> #include <linux/bitops.h>
> +#include <linux/namei.h>
>
> #include "f2fs.h"
> #include "node.h"
> @@ -188,8 +189,18 @@ make_now:
> inode->i_mapping->a_ops = &f2fs_dblock_aops;
> mapping_set_gfp_mask(inode->i_mapping, GFP_F2FS_HIGH_ZERO);
> } else if (S_ISLNK(inode->i_mode)) {
> - inode->i_op = &f2fs_symlink_inode_operations;
> - inode->i_mapping->a_ops = &f2fs_dblock_aops;
> + if (f2fs_inode_is_fast_symlink(inode)) {
> + struct page *node_page;
> +
> + node_page = get_node_page(sbi, inode->i_ino);
> + inode->i_op = &f2fs_fast_symlink_inode_operations;
> + nd_terminate_link(F2FS_INODE(node_page)->i_addr,
> + inode->i_size,
> + sizeof(F2FS_INODE(node_page)->i_addr) - 1);
I don't think nd_terminate_link() is necessary because we have already copied
terminate char '\0' when creating fast symlink.
> + } else {
> + inode->i_op = &f2fs_symlink_inode_operations;
> + inode->i_mapping->a_ops = &f2fs_dblock_aops;
> + }
> } else if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode) ||
> S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode)) {
> inode->i_op = &f2fs_special_inode_operations;
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 1e2ae21..ffc0d52 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -14,6 +14,7 @@
> #include <linux/sched.h>
> #include <linux/ctype.h>
> #include <linux/dcache.h>
> +#include <linux/namei.h>
>
> #include "f2fs.h"
> #include "node.h"
> @@ -247,6 +248,16 @@ fail:
> return err;
> }
>
> +static void *f2fs_follow_link(struct dentry *dentry, struct nameidata *nd)
> +{
> + struct page *node_page;
> +
> + node_page = get_node_page(F2FS_I_SB(dentry->d_inode),
> + dentry->d_inode->i_ino);
get_node_page can fail, we should handle this.
> + nd_set_link(nd, (char *)(F2FS_INODE(node_page)->i_addr));
f2fs_put_page(node_page, 1);
> + return NULL;
> +}
> +
> static int f2fs_symlink(struct inode *dir, struct dentry *dentry,
> const char *symname)
> {
> @@ -254,6 +265,7 @@ static int f2fs_symlink(struct inode *dir, struct dentry
> *dentry,
> struct inode *inode;
> size_t symlen = strlen(symname) + 1;
> int err;
> + struct page *node_page;
>
> f2fs_balance_fs(sbi);
>
> @@ -261,16 +273,28 @@ static int f2fs_symlink(struct inode *dir, struct
> dentry *dentry,
> if (IS_ERR(inode))
> return PTR_ERR(inode);
>
> - inode->i_op = &f2fs_symlink_inode_operations;
> - inode->i_mapping->a_ops = &f2fs_dblock_aops;
>
> + clear_inode_flag(F2FS_I(inode), FI_INLINE_DATA);
> + clear_inode_flag(F2FS_I(inode), FI_INLINE_DENTRY);
These flags can't be set, so we can remove them.
> f2fs_lock_op(sbi);
> err = f2fs_add_link(dentry, inode);
> if (err)
> goto out;
> f2fs_unlock_op(sbi);
>
> - err = page_symlink(inode, symname, symlen);
> + node_page = get_node_page(sbi, inode->i_ino);
move it to below.
> +
> + if (symlen > sizeof(F2FS_INODE(node_page)->i_addr)) {
We always remain space in inode page for inline xattr data,
so it's better to define our max size of fast symlink as below:
#define MAX_FAST_SYMLINK_SIZE (MAX_INLINE_DATA + 1)
> + /* slow symlink */
> + inode->i_op = &f2fs_symlink_inode_operations;
> + inode->i_mapping->a_ops = &f2fs_dblock_aops;
> + err = page_symlink(inode, symname, symlen);
> + } else {
> + /* fast symlink */
> + inode->i_op = &f2fs_fast_symlink_inode_operations;
node_page = get_node_page(sbi, inode->i_ino);
> + memcpy((char *)&F2FS_INODE(node_page)->i_addr, symname, symlen);
set_page_dirty(node_page);
f2fs_put_page(node_page, 1);
> + inode->i_size = symlen-1;
set_inode_flag(F2FS_I(inode), FI_FAST_SYMLINK);
mark_inode_dirty(inode);
Thanks,
> + }
> alloc_nid_done(sbi, inode->i_ino);
>
> d_instantiate(dentry, inode);
> @@ -743,6 +767,20 @@ const struct inode_operations
> f2fs_symlink_inode_operations = {
> #endif
> };
>
> +const struct inode_operations f2fs_fast_symlink_inode_operations = {
> + .readlink = generic_readlink,
> + .follow_link = f2fs_follow_link,
> + .put_link = page_put_link,
> + .getattr = f2fs_getattr,
> + .setattr = f2fs_setattr,
> +#ifdef CONFIG_F2FS_FS_XATTR
> + .setxattr = generic_setxattr,
> + .getxattr = generic_getxattr,
> + .listxattr = f2fs_listxattr,
> + .removexattr = generic_removexattr,
> +#endif
> +};
> +
> const struct inode_operations f2fs_special_inode_operations = {
> .getattr = f2fs_getattr,
> .setattr = f2fs_setattr,
> --
> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
