2013-06-03 10:04:44

by Huajun Li

[permalink] [raw]
Subject: [RFC 0/5] Enable f2fs support inline data

f2fs inode is so large, small files can be stored directly in the inode,
rather than just storing a single block address and storing the data elsewhere.

This RFC patch set is just to enable f2fs support inline data: files less than
about 3.6K can be stored directly in inode block.

TODO: make small dirs inline too.


Haicheng Li (3):
f2fs: Add helper functions and flag to support inline data
f2fs: Add interface for inline data support
f2fs: add tracepoints to debug inline data operations

Huajun Li (2):
f2fs: Handle inline data read and write
f2fs: Key functions to handle inline data

fs/f2fs/Kconfig | 10 +++
fs/f2fs/Makefile | 1 +
fs/f2fs/data.c | 78 +++++++++++++++++++++-
fs/f2fs/f2fs.h | 70 +++++++++++++++++++
fs/f2fs/file.c | 9 ++-
fs/f2fs/inline.c | 156 +++++++++++++++++++++++++++++++++++++++++++
fs/f2fs/inode.c | 8 +++
include/linux/f2fs_fs.h | 5 ++
include/trace/events/f2fs.h | 69 +++++++++++++++++++
9 files changed, 402 insertions(+), 4 deletions(-)
create mode 100644 fs/f2fs/inline.c

--
1.7.9.5


2013-06-03 10:04:58

by Huajun Li

[permalink] [raw]
Subject: [RFC 1/5] f2fs: Add helper functions and flag to support inline data

From: Haicheng Li <[email protected]>

Add a new flag i_dyn_flags to struct f2fs_inode_info to indicate whether
the inode has inline data, and sync the flag with raw inode while update
file.

Signed-off-by: Haicheng Li <[email protected]>
Signed-off-by: Huajun Li <[email protected]>
---
fs/f2fs/f2fs.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++
fs/f2fs/inode.c | 8 ++++++
include/linux/f2fs_fs.h | 5 ++++
3 files changed, 83 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 40b137a..9382f76 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -162,6 +162,7 @@ struct f2fs_inode_info {
umode_t i_acl_mode; /* keep file acl mode temporarily */

/* Use below internally in f2fs*/
+ unsigned long i_dyn_flags; /* use to mark dynamic features */
unsigned long flags; /* use to pass per-file flags */
atomic_t dirty_dents; /* # of dirty dentry pages */
f2fs_hash_t chash; /* hash value of given file name */
@@ -170,6 +171,14 @@ struct f2fs_inode_info {
struct extent_info ext; /* in-memory extent cache entry */
};

+/*
+ * Flags on f2fs_inode_info.i_dyn_flags
+ *
+ * These can change much more often than i_flags.
+ */
+#define F2FS_INLINE_DATA_FL (0x00000001) /* Data stored in inode block */
+#define F2FS_INLINE_DATA_ATTEMPT (0x00000002) /* Data stored in inode block */
+
static inline void get_extent_info(struct extent_info *ext,
struct f2fs_extent i_ext)
{
@@ -877,6 +886,21 @@ static inline void clear_inode_flag(struct f2fs_inode_info *fi, int flag)
clear_bit(flag, &fi->flags);
}

+static inline void set_inode_dyn_flag(struct f2fs_inode_info *fi, int flag)
+{
+ set_bit(flag, &fi->i_dyn_flags);
+}
+
+static inline int is_inode_dyn_flag_set(struct f2fs_inode_info *fi, int flag)
+{
+ return test_bit(flag, &fi->i_dyn_flags);
+}
+
+static inline void clear_inode_dyn_flag(struct f2fs_inode_info *fi, int flag)
+{
+ clear_bit(flag, &fi->i_dyn_flags);
+}
+
static inline void set_acl_inode(struct f2fs_inode_info *fi, umode_t mode)
{
fi->i_acl_mode = mode;
@@ -1042,6 +1066,7 @@ void destroy_checkpoint_caches(void);
* data.c
*/
int reserve_new_block(struct dnode_of_data *);
+int f2fs_reserve_block(struct inode *, pgoff_t);
void update_extent_cache(block_t, struct dnode_of_data *);
struct page *find_data_page(struct inode *, pgoff_t, bool);
struct page *get_lock_data_page(struct inode *, pgoff_t);
@@ -1144,6 +1169,51 @@ static inline void __init f2fs_create_root_stats(void) { }
static inline void f2fs_destroy_root_stats(void) { }
#endif

+/*
+ * inline.c
+ */
+#ifdef CONFIG_F2FS_INLINE_DATA
+#define MAX_INLINE_DATA (sizeof(__le32) * (ADDRS_PER_INODE + 5))
+#define INLINE_DATA_OFFSET (PAGE_CACHE_SIZE - sizeof(struct node_footer)\
+ - MAX_INLINE_DATA)
+
+void f2fs_clear_inode_inline_flag(struct f2fs_inode *);
+void f2fs_set_inode_inline_flag(struct f2fs_inode *);
+int f2fs_inline_data_attempt(struct inode *);
+int f2fs_has_inline_data(struct inode *);
+int f2fs_read_inline_data_page(struct inode *, struct page *);
+int f2fs_convert_inline_data(struct page *, struct inode *, unsigned);
+int f2fs_write_inline_data(struct inode *, struct page *, unsigned int);
+#else
+static inline void
+f2fs_clear_inode_inline_flag(struct f2fs_inode *raw_inode) { }
+static inline void
+f2fs_set_inode_inline_flag(struct f2fs_inode *raw_inode) { }
+int f2fs_inline_data_attempt(struct inode *inode)
+{
+ return 0;
+}
+static inline int f2fs_has_inline_data(struct inode *inode)
+{
+ return 0;
+}
+static inline int f2fs_read_inline_data_page(struct inode *inode,
+ struct page *page)
+{
+ return 0;
+}
+static inline int f2fs_convert_inline_data(struct page *page,
+ struct inode *inode, unsigned flags)
+{
+ return 0;
+}
+static inline int f2fs_write_inline_data(struct inode *inode,
+ struct page *page, unsigned size)
+{
+ return 0;
+}
+#endif
+
extern const struct file_operations f2fs_dir_operations;
extern const struct file_operations f2fs_file_operations;
extern const struct inode_operations f2fs_file_inode_operations;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index b44a4c1..1d7b0b5 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -84,6 +84,9 @@ static int do_read_inode(struct inode *inode)
fi->flags = 0;
fi->i_advise = ri->i_advise;
fi->i_pino = le32_to_cpu(ri->i_pino);
+ if (ri->i_reserved & F2FS_INODE_INLINE_DATA)
+ set_inode_dyn_flag(fi, F2FS_INLINE_DATA_FL);
+
get_extent_info(&fi->ext, ri->i_ext);
f2fs_put_page(node_page, 1);
return 0;
@@ -177,6 +180,11 @@ void update_inode(struct inode *inode, struct page *node_page)
ri->i_pino = cpu_to_le32(F2FS_I(inode)->i_pino);
ri->i_generation = cpu_to_le32(inode->i_generation);

+ if (f2fs_has_inline_data(inode))
+ f2fs_set_inode_inline_flag(ri);
+ else
+ f2fs_clear_inode_inline_flag(ri);
+
if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
if (old_valid_dev(inode->i_rdev)) {
ri->i_addr[0] =
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 383d5e3..505808c 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -176,6 +176,11 @@ struct f2fs_inode {
double_indirect(1) node id */
} __packed;

+/*
+ * Flags on f2fs_inode.i_reserved
+ */
+#define F2FS_INODE_INLINE_DATA 0x01 /* Inode has inline data */
+
struct direct_node {
__le32 addr[ADDRS_PER_BLOCK]; /* array of data block address */
} __packed;
--
1.7.9.5

2013-06-03 10:05:11

by Huajun Li

[permalink] [raw]
Subject: [RFC 3/5] f2fs: Key functions to handle inline data

Functions to implement inline data read/write, and move inline data to
normal data block when file size exceeds inline data limitation.

Signed-off-by: Huajun Li <[email protected]>
Signed-off-by: Haicheng Li <[email protected]>
---
fs/f2fs/inline.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
create mode 100644 fs/f2fs/inline.c

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
new file mode 100644
index 0000000..a2aa056
--- /dev/null
+++ b/fs/f2fs/inline.c
@@ -0,0 +1,152 @@
+/*
+ * fs/f2fs/inline.c
+ *
+ * Copyright (c) 2013, Intel Corporation.
+ * Authors:
+ * Huajun Li <[email protected]>
+ * Haicheng Li <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/fs.h>
+#include <linux/f2fs_fs.h>
+
+#include "f2fs.h"
+
+void f2fs_clear_inode_inline_flag(struct f2fs_inode *raw_inode)
+{
+ raw_inode->i_reserved &= ~F2FS_INODE_INLINE_DATA;
+}
+
+void f2fs_set_inode_inline_flag(struct f2fs_inode *raw_inode)
+{
+ raw_inode->i_reserved |= F2FS_INODE_INLINE_DATA;
+}
+
+int f2fs_inline_data_attempt(struct inode *inode)
+{
+ return is_inode_dyn_flag_set(F2FS_I(inode), F2FS_INLINE_DATA_ATTEMPT);
+}
+
+int f2fs_has_inline_data(struct inode *inode)
+{
+ return is_inode_dyn_flag_set(F2FS_I(inode), F2FS_INLINE_DATA_FL);
+}
+
+static int f2fs_read_inline_data(struct inode *inode, struct page *page)
+{
+ void *src_addr, *dst_addr;
+ loff_t size = i_size_read(inode);
+ struct page *ipage = get_node_page(F2FS_SB(inode->i_sb), inode->i_ino);
+
+ if (IS_ERR(ipage))
+ return PTR_ERR(ipage);
+
+ src_addr = page_address(ipage);
+ dst_addr = page_address(page);
+
+ memcpy(dst_addr, src_addr + INLINE_DATA_OFFSET, size);
+ zero_user_segment(page, INLINE_DATA_OFFSET + size, PAGE_CACHE_SIZE);
+ SetPageUptodate(page);
+
+ f2fs_put_page(ipage, 1);
+
+ return 0;
+}
+
+int f2fs_read_inline_data_page(struct inode *inode, struct page *page)
+{
+ int ret = 0;
+
+ if (!page->index) {
+ ret = f2fs_read_inline_data(inode, page);
+ } else if (!PageUptodate(page)) {
+ zero_user_segment(page, 0, PAGE_CACHE_SIZE);
+ SetPageUptodate(page);
+ }
+
+ unlock_page(page);
+
+ return ret;
+}
+
+int f2fs_convert_inline_data(struct page *p,
+ struct inode *inode, unsigned flags)
+{
+ int err;
+ int ilock;
+ loff_t size;
+ struct page *page, *ipage;
+ void *src_addr, *dst_addr;
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+
+ if (!p->index)
+ page = p;
+ else
+ page = grab_cache_page_write_begin(inode->i_mapping, 0, flags);
+
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+
+ ipage = get_node_page(sbi, inode->i_ino);
+ if (IS_ERR(ipage)) {
+ f2fs_put_page(page, 1);
+ return PTR_ERR(ipage);
+ }
+
+ src_addr = page_address(ipage);
+ dst_addr = page_address(page);
+
+ size = i_size_read(inode);
+ memcpy(dst_addr, src_addr + INLINE_DATA_OFFSET, size);
+ zero_user_segment(ipage, INLINE_DATA_OFFSET,
+ INLINE_DATA_OFFSET + MAX_INLINE_DATA);
+ clear_inode_dyn_flag(F2FS_I(inode), F2FS_INLINE_DATA_FL);
+ set_page_dirty(ipage);
+ f2fs_put_page(ipage, 1);
+
+ if (!p->index) {
+ SetPageUptodate(page);
+ } else {
+ ilock = mutex_lock_op(sbi);
+ err = f2fs_reserve_block(inode, 0);
+ if (err)
+ goto err;
+ mutex_unlock_op(sbi, ilock);
+
+ set_page_dirty(page);
+ f2fs_put_page(page, 1);
+ }
+
+ return 0;
+
+err:
+ mutex_unlock_op(sbi, ilock);
+ f2fs_put_page(page, 1);
+ return err;
+}
+
+int f2fs_write_inline_data(struct inode *inode,
+ struct page *page, unsigned size)
+{
+ void *src_addr, *dst_addr;
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+ struct page *ipage = get_node_page(sbi, inode->i_ino);
+
+ if (IS_ERR(ipage))
+ return PTR_ERR(ipage);
+
+ src_addr = page_address(page);
+ dst_addr = page_address(ipage);
+
+ memcpy(dst_addr + INLINE_DATA_OFFSET, src_addr, size);
+ clear_inode_dyn_flag(F2FS_I(inode), F2FS_INLINE_DATA_ATTEMPT);
+ if (!f2fs_has_inline_data(inode))
+ set_inode_dyn_flag(F2FS_I(inode), F2FS_INLINE_DATA_FL);
+ set_page_dirty(ipage);
+ f2fs_put_page(ipage, 1);
+
+ return 0;
+}
--
1.7.9.5

2013-06-03 10:05:18

by Huajun Li

[permalink] [raw]
Subject: [RFC 5/5] f2fs: add tracepoints to debug inline data operations

From: Haicheng Li <[email protected]>

Add tracepoints for: f2fs_read_inline_data(), f2fs_convert_inline_data(),
f2fs_write_inline_data().

Cc: Steven Rostedt <[email protected]>
Signed-off-by: Haicheng Li <[email protected]>
Signed-off-by: Huajun Li <[email protected]>
---
fs/f2fs/inline.c | 4 +++
include/trace/events/f2fs.h | 69 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index a2aa056..9ec66e5 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -14,6 +14,7 @@
#include <linux/f2fs_fs.h>

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

void f2fs_clear_inode_inline_flag(struct f2fs_inode *raw_inode)
{
@@ -44,6 +45,7 @@ static int f2fs_read_inline_data(struct inode *inode, struct page *page)
if (IS_ERR(ipage))
return PTR_ERR(ipage);

+ trace_f2fs_read_inline_data(inode, page);
src_addr = page_address(ipage);
dst_addr = page_address(page);

@@ -107,6 +109,7 @@ int f2fs_convert_inline_data(struct page *p,
set_page_dirty(ipage);
f2fs_put_page(ipage, 1);

+ trace_f2fs_convert_inline_data(inode, page);
if (!p->index) {
SetPageUptodate(page);
} else {
@@ -138,6 +141,7 @@ int f2fs_write_inline_data(struct inode *inode,
if (IS_ERR(ipage))
return PTR_ERR(ipage);

+ trace_f2fs_write_inline_data(inode, page);
src_addr = page_address(page);
dst_addr = page_address(ipage);

diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 52ae548..bc7a84e 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -676,6 +676,75 @@ TRACE_EVENT(f2fs_write_checkpoint,
__entry->msg)
);

+TRACE_EVENT(f2fs_read_inline_data,
+
+ TP_PROTO(struct inode *inode, struct page *page),
+
+ TP_ARGS(inode, page),
+
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(ino_t, ino)
+ __field(pgoff_t, index)
+ ),
+
+ TP_fast_assign(
+ __entry->dev = inode->i_sb->s_dev;
+ __entry->ino = inode->i_ino;
+ __entry->index = page->index;
+ ),
+
+ TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
+ show_dev_ino(__entry),
+ (unsigned long)__entry->index)
+);
+
+TRACE_EVENT(f2fs_convert_inline_data,
+
+ TP_PROTO(struct inode *inode, struct page *page),
+
+ TP_ARGS(inode, page),
+
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(ino_t, ino)
+ __field(pgoff_t, index)
+ ),
+
+ TP_fast_assign(
+ __entry->dev = inode->i_sb->s_dev;
+ __entry->ino = inode->i_ino;
+ __entry->index = page->index;
+ ),
+
+ TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
+ show_dev_ino(__entry),
+ (unsigned long)__entry->index)
+);
+
+TRACE_EVENT(f2fs_write_inline_data,
+
+ TP_PROTO(struct inode *inode, struct page *page),
+
+ TP_ARGS(inode, page),
+
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(ino_t, ino)
+ __field(pgoff_t, index)
+ ),
+
+ TP_fast_assign(
+ __entry->dev = inode->i_sb->s_dev;
+ __entry->ino = inode->i_ino;
+ __entry->index = page->index;
+ ),
+
+ TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
+ show_dev_ino(__entry),
+ (unsigned long)__entry->index)
+);
+
#endif /* _TRACE_F2FS_H */

/* This part must be outside protection */
--
1.7.9.5

2013-06-03 10:05:50

by Huajun Li

[permalink] [raw]
Subject: [RFC 4/5] f2fs: Add Kconfig interface for inline data support

From: Haicheng Li <[email protected]>

Add Kconfig interface for inline data support, and update Makefile to
compile new added source file.

Signed-off-by: Haicheng Li <[email protected]>
Signed-off-by: Huajun Li <[email protected]>
---
fs/f2fs/Kconfig | 10 ++++++++++
fs/f2fs/Makefile | 1 +
2 files changed, 11 insertions(+)

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index fd27e7e..75da2e4 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -51,3 +51,13 @@ config F2FS_FS_POSIX_ACL
Linux website <http://acl.bestbits.at/>.

If you don't know what Access Control Lists are, say N
+
+config F2FS_INLINE_DATA
+ bool "F2FS inline data support"
+ depends on F2FS_FS
+ default y
+ help
+ Inline data support is an optimization of F2FS data space management,
+ which utilizes the unused space within f2fs inode to keep real data.
+
+ If unsure, say N.
diff --git a/fs/f2fs/Makefile b/fs/f2fs/Makefile
index 27a0820..e1ff7b8 100644
--- a/fs/f2fs/Makefile
+++ b/fs/f2fs/Makefile
@@ -5,3 +5,4 @@ f2fs-y += checkpoint.o gc.o data.o node.o segment.o recovery.o
f2fs-$(CONFIG_F2FS_STAT_FS) += debug.o
f2fs-$(CONFIG_F2FS_FS_XATTR) += xattr.o
f2fs-$(CONFIG_F2FS_FS_POSIX_ACL) += acl.o
+f2fs-$(CONFIG_F2FS_INLINE_DATA) += inline.o
--
1.7.9.5

2013-06-03 10:05:08

by Huajun Li

[permalink] [raw]
Subject: [RFC 2/5] f2fs: Handle inline data read and write

Hook inline data read/write in address space operations.

Signed-off-by: Huajun Li <[email protected]>
Signed-off-by: Haicheng Li <[email protected]>
---
fs/f2fs/data.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
fs/f2fs/file.c | 9 +++++--
2 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 93917e3..bac25f3 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -64,6 +64,23 @@ int reserve_new_block(struct dnode_of_data *dn)
return 0;
}

+int f2fs_reserve_block(struct inode *inode, pgoff_t index)
+{
+ int err;
+ struct dnode_of_data dn;
+
+ set_new_dnode(&dn, inode, NULL, NULL, 0);
+ err = get_dnode_of_data(&dn, index, ALLOC_NODE);
+ if (err)
+ return err;
+ if (dn.data_blkaddr == NULL_ADDR)
+ err = reserve_new_block(&dn);
+
+ f2fs_put_dnode(&dn);
+
+ return err;
+}
+
static int check_extent_cache(struct inode *inode, pgoff_t pgofs,
struct buffer_head *bh_result)
{
@@ -461,13 +478,28 @@ static int get_data_block_ro(struct inode *inode, sector_t iblock,

static int f2fs_read_data_page(struct file *file, struct page *page)
{
- return mpage_readpage(page, get_data_block_ro);
+ int ret;
+ struct inode *inode = file->f_mapping->host;
+
+ /* If the file has inline data, try to read it directlly */
+ if (f2fs_has_inline_data(inode))
+ ret = f2fs_read_inline_data_page(inode, page);
+ else
+ ret = mpage_readpage(page, get_data_block_ro);
+
+ return ret;
}

static int f2fs_read_data_pages(struct file *file,
struct address_space *mapping,
struct list_head *pages, unsigned nr_pages)
{
+ struct inode *inode = file->f_mapping->host;
+
+ /* If the file has inline data, skip readpages */
+ if (f2fs_has_inline_data(inode))
+ return 0;
+
return mpage_readpages(mapping, pages, nr_pages, get_data_block_ro);
}

@@ -517,7 +549,7 @@ static int f2fs_write_data_page(struct page *page,
loff_t i_size = i_size_read(inode);
const pgoff_t end_index = ((unsigned long long) i_size)
>> PAGE_CACHE_SHIFT;
- unsigned offset;
+ unsigned offset = 0;
bool need_balance_fs = false;
int err = 0;

@@ -551,7 +583,16 @@ write:
err = do_write_data_page(page);
} else {
int ilock = mutex_lock_op(sbi);
+
+#ifdef CONFIG_F2FS_INLINE_DATA
+ if (i_size <= MAX_INLINE_DATA)
+ err = f2fs_write_inline_data(inode, page, offset);
+ else
+ err = do_write_data_page(page);
+#else
err = do_write_data_page(page);
+#endif
+
mutex_unlock_op(sbi, ilock);
need_balance_fs = true;
}
@@ -643,6 +684,25 @@ repeat:
return -ENOMEM;
*pagep = page;

+#ifdef CONFIG_F2FS_INLINE_DATA
+ if ((pos + len) <= MAX_INLINE_DATA) {
+ set_inode_dyn_flag(F2FS_I(inode), F2FS_INLINE_DATA_ATTEMPT);
+ goto inline_data;
+ } else if (f2fs_has_inline_data(inode)) {
+ err = f2fs_convert_inline_data(page, inode, flags);
+ if (err)
+ return err;
+ } else if (f2fs_inline_data_attempt(inode)) {
+ clear_inode_dyn_flag(F2FS_I(inode), F2FS_INLINE_DATA_ATTEMPT);
+
+ ilock = mutex_lock_op(sbi);
+ err = f2fs_reserve_block(inode, 0);
+ if (err)
+ goto err;
+ mutex_unlock_op(sbi, ilock);
+ }
+#endif
+
ilock = mutex_lock_op(sbi);

set_new_dnode(&dn, inode, NULL, NULL, 0);
@@ -659,6 +719,7 @@ repeat:

mutex_unlock_op(sbi, ilock);

+inline_data:
if ((len == PAGE_CACHE_SIZE) || PageUptodate(page))
return 0;

@@ -674,7 +735,16 @@ repeat:
if (dn.data_blkaddr == NEW_ADDR) {
zero_user_segment(page, 0, PAGE_CACHE_SIZE);
} else {
+#ifdef CONFIG_F2FS_INLINE_DATA
+ if ((pos + len) <= MAX_INLINE_DATA)
+ err = f2fs_read_inline_data_page(inode, page);
+ else
+ err = f2fs_readpage(sbi, page,
+ dn.data_blkaddr, READ_SYNC);
+#else
err = f2fs_readpage(sbi, page, dn.data_blkaddr, READ_SYNC);
+#endif
+
if (err)
return err;
lock_page(page);
@@ -707,6 +777,10 @@ static ssize_t f2fs_direct_IO(int rw, struct kiocb *iocb,
if (rw == WRITE)
return 0;

+ /* Let buffer I/O handle the inline data case. */
+ if (f2fs_has_inline_data(inode))
+ return 0;
+
/* Needs synchronization with the cleaner */
return blockdev_direct_IO(rw, iocb, inode, iov, offset, nr_segs,
get_data_block_ro);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index deefd25..03b13b3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -178,6 +178,9 @@ int truncate_data_blocks_range(struct dnode_of_data *dn, int count)
raw_node = page_address(dn->node_page);
addr = blkaddr_in_node(raw_node) + ofs;

+ if (f2fs_has_inline_data(dn->inode))
+ return nr_free;
+
for ( ; count > 0; count--, addr++, dn->ofs_in_node++) {
block_t blkaddr = le32_to_cpu(*addr);
if (blkaddr == NULL_ADDR)
@@ -267,11 +270,13 @@ static int truncate_blocks(struct inode *inode, u64 from)

f2fs_put_dnode(&dn);
free_next:
- err = truncate_inode_blocks(inode, free_from);
+ if (!f2fs_has_inline_data(inode))
+ err = truncate_inode_blocks(inode, free_from);
mutex_unlock_op(sbi, ilock);

/* lastly zero out the first data page */
- truncate_partial_data_page(inode, from);
+ if (!f2fs_has_inline_data(inode))
+ truncate_partial_data_page(inode, from);

trace_f2fs_truncate_blocks_exit(inode, err);
return err;
--
1.7.9.5

2013-06-03 13:50:07

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC 5/5] f2fs: add tracepoints to debug inline data operations

On Mon, 2013-06-03 at 18:04 +0800, Huajun Li wrote:
> From: Haicheng Li <[email protected]>
>
> Add tracepoints for: f2fs_read_inline_data(), f2fs_convert_inline_data(),
> f2fs_write_inline_data().
>
> Cc: Steven Rostedt <[email protected]>
> Signed-off-by: Haicheng Li <[email protected]>
> Signed-off-by: Huajun Li <[email protected]>
> ---
> fs/f2fs/inline.c | 4 +++
> include/trace/events/f2fs.h | 69 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index a2aa056..9ec66e5 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -14,6 +14,7 @@
> #include <linux/f2fs_fs.h>
>
> #include "f2fs.h"
> +#include <trace/events/f2fs.h>
>
> void f2fs_clear_inode_inline_flag(struct f2fs_inode *raw_inode)
> {
> @@ -44,6 +45,7 @@ static int f2fs_read_inline_data(struct inode *inode, struct page *page)
> if (IS_ERR(ipage))
> return PTR_ERR(ipage);
>
> + trace_f2fs_read_inline_data(inode, page);
> src_addr = page_address(ipage);
> dst_addr = page_address(page);
>
> @@ -107,6 +109,7 @@ int f2fs_convert_inline_data(struct page *p,
> set_page_dirty(ipage);
> f2fs_put_page(ipage, 1);
>
> + trace_f2fs_convert_inline_data(inode, page);
> if (!p->index) {
> SetPageUptodate(page);
> } else {
> @@ -138,6 +141,7 @@ int f2fs_write_inline_data(struct inode *inode,
> if (IS_ERR(ipage))
> return PTR_ERR(ipage);
>
> + trace_f2fs_write_inline_data(inode, page);
> src_addr = page_address(page);
> dst_addr = page_address(ipage);
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 52ae548..bc7a84e 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -676,6 +676,75 @@ TRACE_EVENT(f2fs_write_checkpoint,
> __entry->msg)
> );
>
> +TRACE_EVENT(f2fs_read_inline_data,
> +
> + TP_PROTO(struct inode *inode, struct page *page),
> +
> + TP_ARGS(inode, page),
> +
> + TP_STRUCT__entry(
> + __field(dev_t, dev)
> + __field(ino_t, ino)
> + __field(pgoff_t, index)
> + ),
> +
> + TP_fast_assign(
> + __entry->dev = inode->i_sb->s_dev;
> + __entry->ino = inode->i_ino;
> + __entry->index = page->index;
> + ),
> +
> + TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
> + show_dev_ino(__entry),
> + (unsigned long)__entry->index)
> +);
> +
> +TRACE_EVENT(f2fs_convert_inline_data,
> +
> + TP_PROTO(struct inode *inode, struct page *page),
> +
> + TP_ARGS(inode, page),
> +
> + TP_STRUCT__entry(
> + __field(dev_t, dev)
> + __field(ino_t, ino)
> + __field(pgoff_t, index)
> + ),
> +
> + TP_fast_assign(
> + __entry->dev = inode->i_sb->s_dev;
> + __entry->ino = inode->i_ino;
> + __entry->index = page->index;
> + ),
> +
> + TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
> + show_dev_ino(__entry),
> + (unsigned long)__entry->index)
> +);
> +
> +TRACE_EVENT(f2fs_write_inline_data,
> +
> + TP_PROTO(struct inode *inode, struct page *page),
> +
> + TP_ARGS(inode, page),
> +
> + TP_STRUCT__entry(
> + __field(dev_t, dev)
> + __field(ino_t, ino)
> + __field(pgoff_t, index)
> + ),
> +
> + TP_fast_assign(
> + __entry->dev = inode->i_sb->s_dev;
> + __entry->ino = inode->i_ino;
> + __entry->index = page->index;
> + ),
> +
> + TP_printk("dev = (%d,%d), ino = %lu, index = %lu",
> + show_dev_ino(__entry),
> + (unsigned long)__entry->index)
> +);

Can you convert the above to use DECLARE_EVENT_CLASS() and
DEFINE_EVENT(), as the above three are basically the same. You'll save a
few K in text by doing so.

Thanks,

-- Steve



> +
> #endif /* _TRACE_F2FS_H */
>
> /* This part must be outside protection */

2013-06-03 23:45:49

by Haicheng Li

[permalink] [raw]
Subject: Re: [RFC 5/5] f2fs: add tracepoints to debug inline data operations

On Mon, Jun 03, 2013 at 09:50:00AM -0400, Steven Rostedt wrote:
> Can you convert the above to use DECLARE_EVENT_CLASS() and
> DEFINE_EVENT(), as the above three are basically the same. You'll save a
> few K in text by doing so.

sure, thank you for the review.

>
> > +
> > #endif /* _TRACE_F2FS_H */
> >
> > /* This part must be outside protection */
>

2013-06-04 02:20:46

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi,

This feature is one of my todo items. ;)
Thank you for the contribution.

Before reviewing the below code intensively, we need to check the
following issues.

- deadlock conditions
- FS consistency
- recovery routine

Could you check one more time?
Thanks again,

2013-06-03 (월), 18:04 +0800, Huajun Li:
> f2fs inode is so large, small files can be stored directly in the inode,
> rather than just storing a single block address and storing the data elsewhere.
>
> This RFC patch set is just to enable f2fs support inline data: files less than
> about 3.6K can be stored directly in inode block.
>
> TODO: make small dirs inline too.
>
>
> Haicheng Li (3):
> f2fs: Add helper functions and flag to support inline data
> f2fs: Add interface for inline data support
> f2fs: add tracepoints to debug inline data operations
>
> Huajun Li (2):
> f2fs: Handle inline data read and write
> f2fs: Key functions to handle inline data
>
> fs/f2fs/Kconfig | 10 +++
> fs/f2fs/Makefile | 1 +
> fs/f2fs/data.c | 78 +++++++++++++++++++++-
> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> fs/f2fs/file.c | 9 ++-
> fs/f2fs/inline.c | 156 +++++++++++++++++++++++++++++++++++++++++++
> fs/f2fs/inode.c | 8 +++
> include/linux/f2fs_fs.h | 5 ++
> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> 9 files changed, 402 insertions(+), 4 deletions(-)
> create mode 100644 fs/f2fs/inline.c
>

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-04 04:24:01

by Namjae Jeon

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi. Huajun.

I agree jaegeuk's opinion.
Additionally, It is better that you describe the effect in change-log
when this feature is added to f2fs.
e.g.
1. how much space is saved when storing kernel-tree(small files) ?
2. small files creation performance test.
3. file look-up performance test.
4. other performance tools 's result.

Thanks.

2013/6/4 Jaegeuk Kim <[email protected]>:
> Hi,
>
> This feature is one of my todo items. ;)
> Thank you for the contribution.
>
> Before reviewing the below code intensively, we need to check the
> following issues.
>
> - deadlock conditions
> - FS consistency
> - recovery routine
>
> Could you check one more time?
> Thanks again,
>
> 2013-06-03 (월), 18:04 +0800, Huajun Li:
>> f2fs inode is so large, small files can be stored directly in the inode,
>> rather than just storing a single block address and storing the data elsewhere.
>>
>> This RFC patch set is just to enable f2fs support inline data: files less than
>> about 3.6K can be stored directly in inode block.
>>
>> TODO: make small dirs inline too.
>>
>>
>> Haicheng Li (3):
>> f2fs: Add helper functions and flag to support inline data
>> f2fs: Add interface for inline data support
>> f2fs: add tracepoints to debug inline data operations
>>
>> Huajun Li (2):
>> f2fs: Handle inline data read and write
>> f2fs: Key functions to handle inline data
>>
>> fs/f2fs/Kconfig | 10 +++
>> fs/f2fs/Makefile | 1 +
>> fs/f2fs/data.c | 78 +++++++++++++++++++++-
>> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
>> fs/f2fs/file.c | 9 ++-
>> fs/f2fs/inline.c | 156 +++++++++++++++++++++++++++++++++++++++++++
>> fs/f2fs/inode.c | 8 +++
>> include/linux/f2fs_fs.h | 5 ++
>> include/trace/events/f2fs.h | 69 +++++++++++++++++++
>> 9 files changed, 402 insertions(+), 4 deletions(-)
>> create mode 100644 fs/f2fs/inline.c
>>
>
> --
> Jaegeuk Kim
> Samsung

2013-06-04 06:01:38

by Haicheng Li

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi Jaegeuk & Namjae,

Sure, we'll address your comments. And this version is RFC, just wanna to
make sure this feature is meaningful for f2fs project, and there is no obvious
mistake, e.g. missing some critical path.

And if you team has some special opensource test suites used in your daily
f2fs test cycle, pls. kindly share the info with us, then we can make sure our
patchset can pass these cases before we send out next version.

BTW, test the kernel source tree or kernel build is a good suggestion. thanks.

On Tue, Jun 04, 2013 at 01:23:57PM +0900, Namjae Jeon wrote:
> Hi. Huajun.
>
> I agree jaegeuk's opinion.
> Additionally, It is better that you describe the effect in change-log
> when this feature is added to f2fs.
> e.g.
> 1. how much space is saved when storing kernel-tree(small files) ?
> 2. small files creation performance test.
> 3. file look-up performance test.
> 4. other performance tools 's result.
>
> Thanks.
>
> 2013/6/4 Jaegeuk Kim <[email protected]>:
> > Hi,
> >
> > This feature is one of my todo items. ;)
> > Thank you for the contribution.
> >
> > Before reviewing the below code intensively, we need to check the
> > following issues.
> >
> > - deadlock conditions
> > - FS consistency
> > - recovery routine
> >
> > Could you check one more time?
> > Thanks again,
> >
> > 2013-06-03 (월), 18:04 +0800, Huajun Li:
> >> f2fs inode is so large, small files can be stored directly in the inode,
> >> rather than just storing a single block address and storing the data elsewhere.
> >>
> >> This RFC patch set is just to enable f2fs support inline data: files less than
> >> about 3.6K can be stored directly in inode block.
> >>
> >> TODO: make small dirs inline too.
> >>
> >>
> >> Haicheng Li (3):
> >> f2fs: Add helper functions and flag to support inline data
> >> f2fs: Add interface for inline data support
> >> f2fs: add tracepoints to debug inline data operations
> >>
> >> Huajun Li (2):
> >> f2fs: Handle inline data read and write
> >> f2fs: Key functions to handle inline data
> >>
> >> fs/f2fs/Kconfig | 10 +++
> >> fs/f2fs/Makefile | 1 +
> >> fs/f2fs/data.c | 78 +++++++++++++++++++++-
> >> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> >> fs/f2fs/file.c | 9 ++-
> >> fs/f2fs/inline.c | 156 +++++++++++++++++++++++++++++++++++++++++++
> >> fs/f2fs/inode.c | 8 +++
> >> include/linux/f2fs_fs.h | 5 ++
> >> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> >> 9 files changed, 402 insertions(+), 4 deletions(-)
> >> create mode 100644 fs/f2fs/inline.c
> >>
> >
> > --
> > Jaegeuk Kim
> > Samsung
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-06-05 07:14:53

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi Haicheng,

2013-06-04 (화), 14:01 +0800, Haicheng Li:
> Hi Jaegeuk & Namjae,
>
> Sure, we'll address your comments. And this version is RFC, just wanna to
> make sure this feature is meaningful for f2fs project, and there is no obvious
> mistake, e.g. missing some critical path.

IMO, it is worth to design and implement the inline feature though, I'd
like to review the total design before looking at trivial mistakes.
Since if the initial design is changed frequently, we need to review
again and again.

So, we need to decide the overall inline design.
Currently we have to consider three data structures at a same time for
the on-disk *inline* inode block structure, which are data, dentry, and
xattr as well.

IMO, we can give three inode flags: F2FS_INLINE_DATA, F2FS_INLINE_DENT,
and F2FS_INLINE_XATTR.

< on-disk inode block >
- metadata
- if F2FS_INLINE_XATTR is set,
: use fixed 2*255 bytes to *cache* two xattrs for simplicity
`- if F2FS_INLINE_DATA is set,
: use the remained space varied by F2FS_INLINE_XATTR.
`- if F2FS_INLINE_DENT is set,
: use variable number of dentries determined by F2FS_INLINE_XATTR.
`- Otherwise, use as pointers

And then, we need to define how to deal with their combinations.

Operational conditions
----------------------
- use inline xattr or not, by checking other inline flags and inline
data size.
- calculate inline data size and its offset according to the use of
inline xattrs.
- the places of inline operaions wrt the lock consistency and coverage
- Power-off-recovery routine should care about the on-disk inode
structure.
- unset F2FS_INLINE_DATA if i_size = 0
- unset F2FS_INLINE_XATTR if xattr entries = 0
- unset F2FS_INLINE_DENT if dentries = 0

- what else?

Once we design the whole thing, we can make general functions to deal
with them gracefully.

> And if you team has some special opensource test suites used in your daily
> f2fs test cycle, pls. kindly share the info with us, then we can make sure our
> patchset can pass these cases before we send out next version.

1. xfstests for functionality
2. fsstress for deadlock/consistency check
3. power-off with fsstress

Thanks,

> BTW, test the kernel source tree or kernel build is a good suggestion. thanks.
>
> On Tue, Jun 04, 2013 at 01:23:57PM +0900, Namjae Jeon wrote:
> > Hi. Huajun.
> >
> > I agree jaegeuk's opinion.
> > Additionally, It is better that you describe the effect in change-log
> > when this feature is added to f2fs.
> > e.g.
> > 1. how much space is saved when storing kernel-tree(small files) ?
> > 2. small files creation performance test.
> > 3. file look-up performance test.
> > 4. other performance tools 's result.
> >
> > Thanks.
> >
> > 2013/6/4 Jaegeuk Kim <[email protected]>:
> > > Hi,
> > >
> > > This feature is one of my todo items. ;)
> > > Thank you for the contribution.
> > >
> > > Before reviewing the below code intensively, we need to check the
> > > following issues.
> > >
> > > - deadlock conditions
> > > - FS consistency
> > > - recovery routine
> > >
> > > Could you check one more time?
> > > Thanks again,
> > >
> > > 2013-06-03 (월), 18:04 +0800, Huajun Li:
> > >> f2fs inode is so large, small files can be stored directly in the inode,
> > >> rather than just storing a single block address and storing the data elsewhere.
> > >>
> > >> This RFC patch set is just to enable f2fs support inline data: files less than
> > >> about 3.6K can be stored directly in inode block.
> > >>
> > >> TODO: make small dirs inline too.
> > >>
> > >>
> > >> Haicheng Li (3):
> > >> f2fs: Add helper functions and flag to support inline data
> > >> f2fs: Add interface for inline data support
> > >> f2fs: add tracepoints to debug inline data operations
> > >>
> > >> Huajun Li (2):
> > >> f2fs: Handle inline data read and write
> > >> f2fs: Key functions to handle inline data
> > >>
> > >> fs/f2fs/Kconfig | 10 +++
> > >> fs/f2fs/Makefile | 1 +
> > >> fs/f2fs/data.c | 78 +++++++++++++++++++++-
> > >> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> > >> fs/f2fs/file.c | 9 ++-
> > >> fs/f2fs/inline.c | 156 +++++++++++++++++++++++++++++++++++++++++++
> > >> fs/f2fs/inode.c | 8 +++
> > >> include/linux/f2fs_fs.h | 5 ++
> > >> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> > >> 9 files changed, 402 insertions(+), 4 deletions(-)
> > >> create mode 100644 fs/f2fs/inline.c
> > >>
> > >
> > > --
> > > Jaegeuk Kim
> > > Samsung
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-09 22:57:15

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi,

2013-06-08 (토), 15:25 +0800, Huajun Li:
>
> Hi Jaegeuk,
> Thanks for your suggestion.
> On Wed, Jun 5, 2013 at 3:13 PM, Jaegeuk Kim <[email protected]>
> wrote:
> Hi Haicheng,
> 2013-06-04 (화), 14:01 +0800, Haicheng Li:
> > Hi Jaegeuk & Namjae,
> >
> > Sure, we'll address your comments. And this version is RFC,
> just wanna to
> > make sure this feature is meaningful for f2fs project, and
> there is no obvious
> > mistake, e.g. missing some critical path.
> IMO, it is worth to design and implement the inline feature
> though, I'd
> like to review the total design before looking at trivial
> mistakes.
> Since if the initial design is changed frequently, we need to
> review
> again and again.
>
> Agree. So let's understand/clarify your following proposal firstly.

> So, we need to decide the overall inline design.
> Currently we have to consider three data structures at a same
> time for
> the on-disk *inline* inode block structure, which are data,
> dentry, and
> xattr as well.
>
> IMO, we can give three inode flags: F2FS_INLINE_DATA,
> F2FS_INLINE_DENT,
> and F2FS_INLINE_XATTR.
>
> Small data and dentries can be inline. And xattr (such as XATTR_USER,
> XATTR_TRUSTED, eg. ) can be inline too, right ?

Right.

> Or inline xattr is just working for inline data/dentries?
>
> < on-disk inode block >
> - metadata
> - if F2FS_INLINE_XATTR is set,
> : use fixed 2*255 bytes to *cache* two xattrs for simplicity
>
> These 2 xattrs are working for the ones storing in xattr_nid block
> before, or just providing info for inline data/dent ?

I meant that we can store a fixed number of xattrs in the inode block
like *inline* data. The number, 2, is just for example.
If users give only one or two xattrs, we can store them in the inode
block instead of using extra node blocks.

If a system enables SELinux, it is able to set security labels to all
the files according to the policy, so that we need to consider inline
xattr seriously.
>
>
> `- if F2FS_INLINE_DATA is set,
> : use the remained space varied by F2FS_INLINE_XATTR.
> `- if F2FS_INLINE_DENT is set,
> : use variable number of dentries determined by
> F2FS_INLINE_XATTR.
> Do you mean inline dent depends on inline xattr, that is, we need a
> dedicated xattr entry providing info for inline denteries, right?

What I meant was the size of inline dentry space. We need to calculate
the size for inline dentries on-the-fly as the F2FS_INLINE_XATTR is set
or not.

>
>
> `- Otherwise, use as pointers
>
> And then, we need to define how to deal with their
> combinations.
>
> Operational conditions
> ----------------------
> - use inline xattr or not, by checking other inline flags and
> inline
> data size.
> - calculate inline data size and its offset according to the
> use of
> inline xattrs.
> - the places of inline operaions wrt the lock consistency and
> coverage
> - Power-off-recovery routine should care about the on-disk
> inode
> structure.
> - unset F2FS_INLINE_DATA if i_size = 0
> - unset F2FS_INLINE_XATTR if xattr entries = 0
> - unset F2FS_INLINE_DENT if dentries = 0
>
> - what else?
> - clear these flags after the inline data/dent is moved to normal data
> block
> - maybe we need store xattr_header while making xattr inline.

It seems that there is nothing to store any header information.

Thanks,

>
> Once we design the whole thing, we can make general functions
> to deal
> with them gracefully.
>
> > And if you team has some special opensource test suites used
> in your daily
> > f2fs test cycle, pls. kindly share the info with us, then we
> can make sure our
> > patchset can pass these cases before we send out next
> version.
>
>
> 1. xfstests for functionality
> 2. fsstress for deadlock/consistency check
> 3. power-off with fsstress
>
> Thanks,
>
> > BTW, test the kernel source tree or kernel build is a good
> suggestion. thanks.
> >
> > On Tue, Jun 04, 2013 at 01:23:57PM +0900, Namjae Jeon wrote:
> > > Hi. Huajun.
> > >
> > > I agree jaegeuk's opinion.
> > > Additionally, It is better that you describe the effect in
> change-log
> > > when this feature is added to f2fs.
> > > e.g.
> > > 1. how much space is saved when storing
> kernel-tree(small files) ?
> > > 2. small files creation performance test.
> > > 3. file look-up performance test.
> > > 4. other performance tools 's result.
> > >
> > > Thanks.
> > >
> > > 2013/6/4 Jaegeuk Kim <[email protected]>:
> > > > Hi,
> > > >
> > > > This feature is one of my todo items. ;)
> > > > Thank you for the contribution.
> > > >
> > > > Before reviewing the below code intensively, we need to
> check the
> > > > following issues.
> > > >
> > > > - deadlock conditions
> > > > - FS consistency
> > > > - recovery routine
> > > >
> > > > Could you check one more time?
> > > > Thanks again,
> > > >
> > > > 2013-06-03 (월), 18:04 +0800, Huajun Li:
> > > >> f2fs inode is so large, small files can be stored
> directly in the inode,
> > > >> rather than just storing a single block address and
> storing the data elsewhere.
> > > >>
> > > >> This RFC patch set is just to enable f2fs support
> inline data: files less than
> > > >> about 3.6K can be stored directly in inode block.
> > > >>
> > > >> TODO: make small dirs inline too.
> > > >>
> > > >>
> > > >> Haicheng Li (3):
> > > >> f2fs: Add helper functions and flag to support inline
> data
> > > >> f2fs: Add interface for inline data support
> > > >> f2fs: add tracepoints to debug inline data operations
> > > >>
> > > >> Huajun Li (2):
> > > >> f2fs: Handle inline data read and write
> > > >> f2fs: Key functions to handle inline data
> > > >>
> > > >> fs/f2fs/Kconfig | 10 +++
> > > >> fs/f2fs/Makefile | 1 +
> > > >> fs/f2fs/data.c | 78
> +++++++++++++++++++++-
> > > >> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> > > >> fs/f2fs/file.c | 9 ++-
> > > >> fs/f2fs/inline.c | 156
> +++++++++++++++++++++++++++++++++++++++++++
> > > >> fs/f2fs/inode.c | 8 +++
> > > >> include/linux/f2fs_fs.h | 5 ++
> > > >> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> > > >> 9 files changed, 402 insertions(+), 4 deletions(-)
> > > >> create mode 100644 fs/f2fs/inline.c
> > > >>
> > > >
> > > > --
> > > > Jaegeuk Kim
> > > > Samsung
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> linux-fsdevel" in
> > > the body of a message to [email protected]
> > > More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
>
> --
> Jaegeuk Kim
> Samsung
>
>

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-08-07 11:37:25

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [RFC 0/5] Enable f2fs support inline data

Hi Huajun,

Sorry for the long delay.
I've been too busy to catch up this new big feature.

Anyway, do you guys still design or focus on this issue?
Nowadays, I can afford to dive into this issue.
So, if you have done any progress so far, can you share it with me?
Otherwise, I'd like to start to design it from the scatch.

Thanks,

2013-06-08, 15:25 +0800, Huajun Li:
>
> Hi Jaegeuk,
> Thanks for your suggestion.
>
>
> On Wed, Jun 5, 2013 at 3:13 PM, Jaegeuk Kim <[email protected]>
> wrote:
> Hi Haicheng,
>
> 2013-06-04 (화), 14:01 +0800, Haicheng Li:
> > Hi Jaegeuk & Namjae,
> >
> > Sure, we'll address your comments. And this version is RFC,
> just wanna to
> > make sure this feature is meaningful for f2fs project, and
> there is no obvious
> > mistake, e.g. missing some critical path.
>
>
> IMO, it is worth to design and implement the inline feature
> though, I'd
> like to review the total design before looking at trivial
> mistakes.
> Since if the initial design is changed frequently, we need to
> review
> again and again.
>
> Agree. So let's understand/clarify your following proposal firstly.
>
>
> So, we need to decide the overall inline design.
> Currently we have to consider three data structures at a same
> time for
> the on-disk *inline* inode block structure, which are data,
> dentry, and
> xattr as well.
>
> IMO, we can give three inode flags: F2FS_INLINE_DATA,
> F2FS_INLINE_DENT,
> and F2FS_INLINE_XATTR.
>
> Small data and dentries can be inline. And xattr (such as XATTR_USER,
> XATTR_TRUSTED, eg. ) can be inline too, right ?
> Or inline xattr is just working for inline data/dentries?
>
> < on-disk inode block >
> - metadata
> - if F2FS_INLINE_XATTR is set,
> : use fixed 2*255 bytes to *cache* two xattrs for simplicity
>
> These 2 xattrs are working for the ones storing in xattr_nid block
> before, or just providing info for inline data/dent ?
>
>
> `- if F2FS_INLINE_DATA is set,
> : use the remained space varied by F2FS_INLINE_XATTR.
> `- if F2FS_INLINE_DENT is set,
> : use variable number of dentries determined by
> F2FS_INLINE_XATTR.
> Do you mean inline dent depends on inline xattr, that is, we need a
> dedicated xattr entry providing info for inline denteries, right?
>
>
> `- Otherwise, use as pointers
>
> And then, we need to define how to deal with their
> combinations.
>
> Operational conditions
> ----------------------
> - use inline xattr or not, by checking other inline flags and
> inline
> data size.
> - calculate inline data size and its offset according to the
> use of
> inline xattrs.
> - the places of inline operaions wrt the lock consistency and
> coverage
> - Power-off-recovery routine should care about the on-disk
> inode
> structure.
> - unset F2FS_INLINE_DATA if i_size = 0
> - unset F2FS_INLINE_XATTR if xattr entries = 0
> - unset F2FS_INLINE_DENT if dentries = 0
>
> - what else?
> - clear these flags after the inline data/dent is moved to normal data
> block
> - maybe we need store xattr_header while making xattr inline.
>
>
>
> Once we design the whole thing, we can make general functions
> to deal
> with them gracefully.
>
> > And if you team has some special opensource test suites used
> in your daily
> > f2fs test cycle, pls. kindly share the info with us, then we
> can make sure our
> > patchset can pass these cases before we send out next
> version.
>
>
> 1. xfstests for functionality
> 2. fsstress for deadlock/consistency check
> 3. power-off with fsstress
>
> Thanks,
>
> > BTW, test the kernel source tree or kernel build is a good
> suggestion. thanks.
> >
> > On Tue, Jun 04, 2013 at 01:23:57PM +0900, Namjae Jeon wrote:
> > > Hi. Huajun.
> > >
> > > I agree jaegeuk's opinion.
> > > Additionally, It is better that you describe the effect in
> change-log
> > > when this feature is added to f2fs.
> > > e.g.
> > > 1. how much space is saved when storing
> kernel-tree(small files) ?
> > > 2. small files creation performance test.
> > > 3. file look-up performance test.
> > > 4. other performance tools 's result.
> > >
> > > Thanks.
> > >
> > > 2013/6/4 Jaegeuk Kim <[email protected]>:
> > > > Hi,
> > > >
> > > > This feature is one of my todo items. ;)
> > > > Thank you for the contribution.
> > > >
> > > > Before reviewing the below code intensively, we need to
> check the
> > > > following issues.
> > > >
> > > > - deadlock conditions
> > > > - FS consistency
> > > > - recovery routine
> > > >
> > > > Could you check one more time?
> > > > Thanks again,
> > > >
> > > > 2013-06-03 (월), 18:04 +0800, Huajun Li:
> > > >> f2fs inode is so large, small files can be stored
> directly in the inode,
> > > >> rather than just storing a single block address and
> storing the data elsewhere.
> > > >>
> > > >> This RFC patch set is just to enable f2fs support
> inline data: files less than
> > > >> about 3.6K can be stored directly in inode block.
> > > >>
> > > >> TODO: make small dirs inline too.
> > > >>
> > > >>
> > > >> Haicheng Li (3):
> > > >> f2fs: Add helper functions and flag to support inline
> data
> > > >> f2fs: Add interface for inline data support
> > > >> f2fs: add tracepoints to debug inline data operations
> > > >>
> > > >> Huajun Li (2):
> > > >> f2fs: Handle inline data read and write
> > > >> f2fs: Key functions to handle inline data
> > > >>
> > > >> fs/f2fs/Kconfig | 10 +++
> > > >> fs/f2fs/Makefile | 1 +
> > > >> fs/f2fs/data.c | 78
> +++++++++++++++++++++-
> > > >> fs/f2fs/f2fs.h | 70 +++++++++++++++++++
> > > >> fs/f2fs/file.c | 9 ++-
> > > >> fs/f2fs/inline.c | 156
> +++++++++++++++++++++++++++++++++++++++++++
> > > >> fs/f2fs/inode.c | 8 +++
> > > >> include/linux/f2fs_fs.h | 5 ++
> > > >> include/trace/events/f2fs.h | 69 +++++++++++++++++++
> > > >> 9 files changed, 402 insertions(+), 4 deletions(-)
> > > >> create mode 100644 fs/f2fs/inline.c
> > > >>
> > > >
> > > > --
> > > > Jaegeuk Kim
> > > > Samsung
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> linux-fsdevel" in
> > > the body of a message to [email protected]
> > > More majordomo info at
> http://vger.kernel.org/majordomo-info.html
>
>
> --
> Jaegeuk Kim
> Samsung
>
>

--
Jaegeuk Kim
Samsung