Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752227AbdHHLmL (ORCPT ); Tue, 8 Aug 2017 07:42:11 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:3030 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752049AbdHHLmK (ORCPT ); Tue, 8 Aug 2017 07:42:10 -0400 Subject: Re: [PATCH 1/1] f2fs: merge equivalent flags F2FS_GET_BLOCK_[READ|DIO] To: Chao Yu , , , References: <1502188039-25453-1-git-send-email-sunqiuyang@huawei.com> From: Sun Qiuyang Message-ID: <8ce0003f-780a-a349-e30e-218fdf61488d@huawei.com> Date: Tue, 8 Aug 2017 19:41:58 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.249.127] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.5989A390.023C,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: 662bd470874f192b0834a36a11ddff33 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2622 Lines: 87 > On 2017/8/8 18:27, sunqiuyang wrote: >> From: Qiuyang Sun >> >> Currently, the two flags F2FS_GET_BLOCK_[READ|DIO] are totally equivalent >> and can be used interchangably in all scenarios they are involved in. This >> patch deletes F2FS_GET_BLOCK_READ and uses F2FS_GET_BLOCK_DIO instead. > > Seems weird from readability point of view, so how about keeping > F2FS_GET_BLOCK_READ alive but sharing defined value with F2FS_GET_BLOCK_DIO? > > enum { > F2FS_GET_BLOCK_DIO, > F2FS_GET_BLOCK_READ = F2FS_GET_BLOCK_DIO, > F2FS_GET_BLOCK_FIEMAP, > ... > } > > Thanks, How about renaming both of them F2FS_GET_BLOCK_DEFAULT? Actually neither READ nor DIO is explicitly referenced in f2fs_map_blocks(); it is just the "default" case. We have no reason to limit the use of this flag to "read" or "direct IO" only. Thanks, > >> >> Signed-off-by: Qiuyang Sun >> --- >> fs/f2fs/data.c | 2 +- >> fs/f2fs/f2fs.h | 1 - >> fs/f2fs/file.c | 4 ++-- >> 3 files changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index c43262d..e0a59bf 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -1244,7 +1244,7 @@ static int f2fs_mpage_readpages(struct address_space *mapping, >> map.m_len = last_block - block_in_file; >> >> if (f2fs_map_blocks(inode, &map, 0, >> - F2FS_GET_BLOCK_READ)) >> + F2FS_GET_BLOCK_DIO)) >> goto set_error_page; >> } >> got_it: >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index cea329f..0593ca7 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -514,7 +514,6 @@ struct f2fs_map_blocks { >> }; >> >> /* for flag in get_data_block */ >> -#define F2FS_GET_BLOCK_READ 0 >> #define F2FS_GET_BLOCK_DIO 1 >> #define F2FS_GET_BLOCK_FIEMAP 2 >> #define F2FS_GET_BLOCK_BMAP 3 >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index e2b33b8..8271cb5 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -2074,7 +2074,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, >> */ >> while (map.m_lblk < pg_end) { >> map.m_len = pg_end - map.m_lblk; >> - err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_READ); >> + err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_DIO); >> if (err) >> goto out; >> >> @@ -2116,7 +2116,7 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, >> >> do_map: >> map.m_len = pg_end - map.m_lblk; >> - err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_READ); >> + err = f2fs_map_blocks(inode, &map, 0, F2FS_GET_BLOCK_DIO); >> if (err) >> goto clear_out; >> >> > > > . >