From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751358AbdKMBYS (ORCPT ); Sun, 12 Nov 2017 20:24:18 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:10521 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751092AbdKMBYQ (ORCPT ); Sun, 12 Nov 2017 20:24:16 -0500 Subject: Re: [f2fs-dev] [RFC PATHC 2/2] f2fs: apply write hints to select the type of segment for direct write To: Hyunchul Lee , Chao Yu , Jaegeuk Kim CC: , Hyunchul Lee , , References: <1510206688-12767-1-git-send-email-hyc.lee@gmail.com> <1510206688-12767-3-git-send-email-hyc.lee@gmail.com> <711d3991-47e1-bc1c-4eb8-c9254fec156a@kernel.org> <5A08E253.50408@gmail.com> From: Chao Yu Message-ID: <70bf8e31-9b5c-e2cc-107b-d699c02c843f@huawei.com> Date: Mon, 13 Nov 2017 09:24:06 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <5A08E253.50408@gmail.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.134.22.195] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020206.5A08F439.005F,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 3bc520ea46eb5df4b1fb02135cbe1536 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/11/13 8:07, Hyunchul Lee wrote: > On 11/11/2017 09:38 AM, Chao Yu wrote: >> On 2017/11/9 13:51, Hyunchul Lee wrote: >>> From: Hyunchul Lee >>> >>> Select the type of the segment using write hints, when blocks are >>> allocated for direct write. >>> >>> There are unhandled corner cases. Hints are not applied in >>> in-place update. And if the blocks of a file is not pre-allocated >>> because of the invalid user buffer, CURSEG_WARM_DATA segment will >>> be selected. >>> >>> Signed-off-by: Hyunchul Lee >>> --- >>> fs/f2fs/data.c | 101 ++++++++++++++++++++++++++++++++++----------------------- >>> fs/f2fs/f2fs.h | 1 + >>> 2 files changed, 61 insertions(+), 41 deletions(-) >>> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>> index 36b5352..d06048a 100644 >>> --- a/fs/f2fs/data.c >>> +++ b/fs/f2fs/data.c >>> @@ -783,7 +783,7 @@ struct page *get_new_data_page(struct inode *inode, >>> return page; >>> } >>> >>> -static int __allocate_data_block(struct dnode_of_data *dn) >>> +static int __allocate_data_block(struct dnode_of_data *dn, int seg_type) >>> { >>> struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode); >>> struct f2fs_summary sum; >>> @@ -808,7 +808,7 @@ static int __allocate_data_block(struct dnode_of_data *dn) >>> set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version); >>> >>> allocate_data_block(sbi, NULL, dn->data_blkaddr, &dn->data_blkaddr, >>> - &sum, CURSEG_WARM_DATA, NULL, false); >>> + &sum, seg_type, NULL, false); >>> set_data_blkaddr(dn); >>> >>> /* update i_size */ >>> @@ -827,42 +827,6 @@ static inline bool __force_buffered_io(struct inode *inode, int rw) >>> F2FS_I_SB(inode)->s_ndevs); >>> } >>> >>> -int f2fs_preallocate_blocks(struct kiocb *iocb, struct iov_iter *from) >>> -{ >>> - struct inode *inode = file_inode(iocb->ki_filp); >>> - struct f2fs_map_blocks map; >>> - int err = 0; >>> - >>> - if (is_inode_flag_set(inode, FI_NO_PREALLOC)) >>> - return 0; >>> - >>> - map.m_lblk = F2FS_BLK_ALIGN(iocb->ki_pos); >>> - map.m_len = F2FS_BYTES_TO_BLK(iocb->ki_pos + iov_iter_count(from)); >>> - if (map.m_len > map.m_lblk) >>> - map.m_len -= map.m_lblk; >>> - else >>> - map.m_len = 0; >>> - >>> - map.m_next_pgofs = NULL; >>> - >>> - if (iocb->ki_flags & IOCB_DIRECT) { >>> - err = f2fs_convert_inline_inode(inode); >>> - if (err) >>> - return err; >>> - return f2fs_map_blocks(inode, &map, 1, >>> - __force_buffered_io(inode, WRITE) ? >>> - F2FS_GET_BLOCK_PRE_AIO : >>> - F2FS_GET_BLOCK_PRE_DIO); >>> - } >>> - if (iocb->ki_pos + iov_iter_count(from) > MAX_INLINE_DATA(inode)) { >>> - err = f2fs_convert_inline_inode(inode); >>> - if (err) >>> - return err; >>> - } >>> - if (!f2fs_has_inline_data(inode)) >>> - return f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO); >>> - return err; >>> -} >>> >>> static inline void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock) >>> { >>> @@ -888,8 +852,8 @@ static inline void __do_map_lock(struct f2fs_sb_info *sbi, int flag, bool lock) >>> * b. do not use extent cache for better performance >>> * c. give the block addresses to blockdev >>> */ >>> -int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >>> - int create, int flag) >>> +static int __f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >>> + int create, int flag, int seg_type) >>> { >>> unsigned int maxblocks = map->m_len; >>> struct dnode_of_data dn; >>> @@ -957,7 +921,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >>> last_ofs_in_node = dn.ofs_in_node; >>> } >>> } else { >>> - err = __allocate_data_block(&dn); >>> + /* if this inode is marked with FI_NO_PREALLOC, >>> + * @seg_type is NO_CHECK_TYPE >>> + */ >>> + if (seg_type == NO_CHECK_TYPE) >>> + seg_type = CURSEG_WARM_DATA; >>> + err = __allocate_data_block(&dn, seg_type); >> >> We need to use inode.i_write_hint instead of ki_hint passed from file.f_write_hint? >> > > The following commit says to use file.f_write_hint if it is available. > "c75b1d9 fs: add fcntl() interface for setting/getting write life time hints" Oh, yes, f_write_hint is recommended. So, I'm OK with this. One left question as below. > > And ki_hint is assiged to file.f_write_hint or inode.i_write_hint > by file_write_hint() in init_sync_kiocb(). > So, I think we need to use ki_hint instead of inode.i_write_hint. > > Thanks > >> Thanks, >> >>> if (!err) >>> set_inode_flag(inode, FI_APPEND_WRITE); >>> } >>> @@ -1048,6 +1017,51 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >>> return err; >>> } >>> >>> +int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >>> + int create, int flag) >>> +{ >>> + return __f2fs_map_blocks(inode, map, create, flag, NO_CHECK_TYPE); >>> +} >>> + >>> +int f2fs_preallocate_blocks(struct kiocb *iocb, struct iov_iter *from) >>> +{ >>> + struct inode *inode = file_inode(iocb->ki_filp); >>> + struct f2fs_map_blocks map; >>> + int err = 0; >>> + >>> + if (is_inode_flag_set(inode, FI_NO_PREALLOC)) >>> + return 0; >>> + >>> + map.m_lblk = F2FS_BLK_ALIGN(iocb->ki_pos); >>> + map.m_len = F2FS_BYTES_TO_BLK(iocb->ki_pos + iov_iter_count(from)); >>> + if (map.m_len > map.m_lblk) >>> + map.m_len -= map.m_lblk; >>> + else >>> + map.m_len = 0; >>> + >>> + map.m_next_pgofs = NULL; >>> + >>> + if (iocb->ki_flags & IOCB_DIRECT) { >>> + err = f2fs_convert_inline_inode(inode); >>> + if (err) >>> + return err; >>> + return __f2fs_map_blocks(inode, &map, 1, >>> + __force_buffered_io(inode, WRITE) ? >>> + F2FS_GET_BLOCK_PRE_AIO : >>> + F2FS_GET_BLOCK_PRE_DIO, >>> + rw_hint_to_seg_type(iocb->ki_hint)); >>> + } >>> + if (iocb->ki_pos + iov_iter_count(from) > MAX_INLINE_DATA(inode)) { >>> + err = f2fs_convert_inline_inode(inode); >>> + if (err) >>> + return err; >>> + } >>> + if (!f2fs_has_inline_data(inode)) >>> + return f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_AIO); >>> + >>> + return err; >>> +} >>> + >>> static int __get_data_block(struct inode *inode, sector_t iblock, >>> struct buffer_head *bh, int create, int flag, >>> pgoff_t *next_pgofs) >>> @@ -2082,6 +2096,11 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) >>> >>> trace_f2fs_direct_IO_enter(inode, offset, count, rw); >>> >>> + /* This is for avoiding the situation that the data of a segment is >>> + * passed down to devices with different hints >>> + */ >>> + iocb->ki_hint = WRITE_LIFE_NOT_SET; Why we need to change this? I'm not sure this will be used later in somewhere, if it's not necessary, how about keeping it as it is? Thanks, >>> + >>> down_read(&F2FS_I(inode)->dio_rwsem[rw]); >>> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); >>> up_read(&F2FS_I(inode)->dio_rwsem[rw]); >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 4b4a72f..9be5658 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -2562,6 +2562,7 @@ int lookup_journal_in_cursum(struct f2fs_journal *journal, int type, >>> void destroy_segment_manager(struct f2fs_sb_info *sbi); >>> int __init create_segment_manager_caches(void); >>> void destroy_segment_manager_caches(void); >>> +int rw_hint_to_seg_type(enum rw_hint hint); >>> >>> /* >>> * checkpoint.c >>> >> > > . >