Hi Jaegeuk,
On 2016/5/10 7:00, Jaegeuk Kim wrote:
> Hi Chao,
>
> On Mon, May 09, 2016 at 07:56:30PM +0800, Chao Yu wrote:
>> This patch introduces reserve_new_blocks to make preallocation of multi
>> blocks as in batch operation, so it can avoid lots of redundant
>> operation, result in better performance.
>>
>> In virtual machine, with rotational device:
>>
>> time fallocate -l 32G /mnt/f2fs/file
>>
>> Before:
>> real 0m4.584s
>> user 0m0.000s
>> sys 0m4.580s
>>
>> After:
>> real 0m0.292s
>> user 0m0.000s
>> sys 0m0.272s
>
> It's cool.
> Let me add my test results as well.
>
> In x86, with SSD:
>
> time fallocate -l 500G $MNT/testfile
>
> Before : 24.758 s
> After : 1.604 s
>
> By the way, there is one thing we should consider, which is the ENOSPC case.
> Could you check this out on top of your patch?
>
> If you don't mind, let me integrate this into your patch.
No problem. :)
And see below comments please.
> Let me know.
>
> Thanks,
>
> ---
> fs/f2fs/data.c | 9 +++++----
> fs/f2fs/f2fs.h | 20 +++++++++++++-------
> include/trace/events/f2fs.h | 8 ++++----
> 3 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index ea0abdc..da640e1 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -278,7 +278,7 @@ alloc_new:
> trace_f2fs_submit_page_mbio(fio->page, fio);
> }
>
> -void __set_data_blkaddr(struct dnode_of_data *dn)
> +static void __set_data_blkaddr(struct dnode_of_data *dn)
> {
> struct f2fs_node *rn = F2FS_NODE(dn->node_page);
> __le32 *addr_array;
> @@ -310,7 +310,7 @@ void f2fs_update_data_blkaddr(struct dnode_of_data *dn,
> block_t blkaddr)
> }
>
> int reserve_new_blocks(struct dnode_of_data *dn, unsigned int start,
> - unsigned int count)
> + blkcnt_t count)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> unsigned int ofs_in_node;
> @@ -320,7 +320,7 @@ int reserve_new_blocks(struct dnode_of_data *dn, unsigned
> int start,
>
> if (unlikely(is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC)))
> return -EPERM;
> - if (unlikely(!inc_valid_block_count(sbi, dn->inode, count)))
> + if (unlikely(!inc_valid_block_count(sbi, dn->inode, &count)))
> return -ENOSPC;
>
> trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
> @@ -574,6 +574,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
> struct node_info ni;
> int seg = CURSEG_WARM_DATA;
> pgoff_t fofs;
> + blkcnt_t count = 1;
>
> if (unlikely(is_inode_flag_set(F2FS_I(dn->inode), FI_NO_ALLOC)))
> return -EPERM;
> @@ -582,7 +583,7 @@ static int __allocate_data_block(struct dnode_of_data *dn)
> if (dn->data_blkaddr == NEW_ADDR)
> goto alloc;
>
> - if (unlikely(!inc_valid_block_count(sbi, dn->inode, 1)))
> + if (unlikely(!inc_valid_block_count(sbi, dn->inode, &count)))
> return -ENOSPC;
>
> alloc:
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 75b0084..00fe63c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1114,7 +1114,7 @@ static inline bool f2fs_has_xattr_block(unsigned int
> ofs)
> }
>
> static inline bool inc_valid_block_count(struct f2fs_sb_info *sbi,
> - struct inode *inode, blkcnt_t count)
> + struct inode *inode, blkcnt_t *count)
> {
> block_t valid_block_count;
>
> @@ -1126,14 +1126,19 @@ static inline bool inc_valid_block_count(struct
> f2fs_sb_info *sbi,
> }
> #endif
> valid_block_count =
> - sbi->total_valid_block_count + (block_t)count;
> + sbi->total_valid_block_count + (block_t)(*count);
> if (unlikely(valid_block_count > sbi->user_block_count)) {
> - spin_unlock(&sbi->stat_lock);
> - return false;
> + *count = sbi->user_block_count - sbi->total_valid_block_count;
> + if (!*count) {
> + spin_unlock(&sbi->stat_lock);
> + return false;
> + }
If we can only allocate partial blocks, we should let f2fs_map_blocks being
ware of that, otherwise, map->m_len will be updated incorrectly.
Thanks,
> }
> - inode->i_blocks += count;
> - sbi->total_valid_block_count = valid_block_count;
> - sbi->alloc_valid_block_count += (block_t)count;
> + /* *count can be recalculated */
> + inode->i_blocks += *count;
> + sbi->total_valid_block_count =
> + sbi->total_valid_block_count + (block_t)(*count);
> + sbi->alloc_valid_block_count += (block_t)(*count);
> spin_unlock(&sbi->stat_lock);
> return true;
> }
> @@ -1965,6 +1970,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *);
> void f2fs_submit_page_mbio(struct f2fs_io_info *);
> void set_data_blkaddr(struct dnode_of_data *);
> void f2fs_update_data_blkaddr(struct dnode_of_data *, block_t);
> +int reserve_new_blocks(struct dnode_of_data *, unsigned int, blkcnt_t);
> int reserve_new_block(struct dnode_of_data *);
> int f2fs_get_block(struct dnode_of_data *, pgoff_t);
> ssize_t f2fs_preallocate_blocks(struct kiocb *, struct iov_iter *);
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 5f927ff..497e6e8 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -697,7 +697,7 @@ TRACE_EVENT(f2fs_direct_IO_exit,
> TRACE_EVENT(f2fs_reserve_new_blocks,
>
> TP_PROTO(struct inode *inode, nid_t nid, unsigned int ofs_in_node,
> - unsigned int count),
> + blkcnt_t count),
>
> TP_ARGS(inode, nid, ofs_in_node, count),
>
> @@ -705,7 +705,7 @@ TRACE_EVENT(f2fs_reserve_new_blocks,
> __field(dev_t, dev)
> __field(nid_t, nid)
> __field(unsigned int, ofs_in_node)
> - __field(unsigned int, count)
> + __field(blkcnt_t, count)
> ),
>
> TP_fast_assign(
> @@ -715,11 +715,11 @@ TRACE_EVENT(f2fs_reserve_new_blocks,
> __entry->count = count;
> ),
>
> - TP_printk("dev = (%d,%d), nid = %u, ofs_in_node = %u, count = %u",
> + TP_printk("dev = (%d,%d), nid = %u, ofs_in_node = %u, count = %llu",
> show_dev(__entry),
> (unsigned int)__entry->nid,
> __entry->ofs_in_node,
> - __entry->count)
> + (unsigned long long)__entry->count)
> );
>
> DECLARE_EVENT_CLASS(f2fs__submit_page_bio,
>
------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
