From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752103AbdJZI7F (ORCPT ); Thu, 26 Oct 2017 04:59:05 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:9514 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751009AbdJZI7B (ORCPT ); Thu, 26 Oct 2017 04:59:01 -0400 Subject: Re: [PATCH] f2fs: fix to keep backward compatibility of flexible inline xattr feature To: Jaegeuk Kim CC: , , References: <20171025103124.63162-1-yuchao0@huawei.com> <20171026084243.GA54331@jaegeuk-macbookpro.roam.corp.google.com> From: Chao Yu Message-ID: <37f397bd-fa5b-c7f0-bb20-ea3be98437cd@huawei.com> Date: Thu, 26 Oct 2017 16:57:38 +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: <20171026084243.GA54331@jaegeuk-macbookpro.roam.corp.google.com> Content-Type: text/plain; charset="windows-1252" 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.0A020201.59F1A3C3.0044,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: 1d41993ec28f8c3e2198981eb00e0af3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jaegeuk, On 2017/10/26 16:42, Jaegeuk Kim wrote: > Hi Chao, > > It seems this is a critical problem, so let me integrate this patch with your > initial patch "f2fs: support flexible inline xattr size". > Let me know, if you have any other concern. Better. ;) Please add commit message of this patch into initial patch "f2fs: support flexible inline xattr size". And please fix related issue in f2fs-tools with below diff code: --- include/f2fs_fs.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h index 2db7495804fd..a0e15ba85d97 100644 --- a/include/f2fs_fs.h +++ b/include/f2fs_fs.h @@ -1047,11 +1047,14 @@ static inline int __get_extra_isize(struct f2fs_inode *inode) extern struct f2fs_configuration c; static inline int get_inline_xattr_addrs(struct f2fs_inode *inode) { - if (!(inode->i_inline & F2FS_INLINE_XATTR)) + if ((c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) && + (c.feature & cpu_to_le32(F2FS_FEATURE_FLEXIBLE_INLINE_XATTR))) + return le16_to_cpu(inode->i_inline_xattr_size); + else if (!(inode->i_inline & F2FS_INLINE_XATTR) && + (S_ISREG(inode->i_mode) || S_ISLNK(inode->i_mode))) return 0; - if (!(c.feature & cpu_to_le32(F2FS_FEATURE_FLEXIBLE_INLINE_XATTR))) + else return DEFAULT_INLINE_XATTR_ADDRS; - return le16_to_cpu(inode->i_inline_xattr_size); } #define get_extra_isize(node) __get_extra_isize(&node->i) -- Thanks, > > Thanks, > > On 10/25, Chao Yu wrote: >> Previously, in inode layout, we will always reserve 200 bytes for inline >> xattr space no matter the inode enables inline xattr feature or not, due >> to this reason, max inline size of inode is fixed, but now, if inline >> xattr is not enabled, max inline size of inode will be enlarged by 200 >> bytes, for regular and symlink inode, it will be safe to reuse resevered >> space as they are all zero, but for directory, we need to keep the >> reservation for stablizing directory structure. >> >> Reported-by: Sheng Yong >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/f2fs.h | 5 +---- >> fs/f2fs/inode.c | 15 ++++++++++++--- >> fs/f2fs/namei.c | 2 ++ >> 3 files changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 2af1d31ae74b..7ddd0d085e3b 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -2393,10 +2393,7 @@ static inline int get_extra_isize(struct inode *inode) >> static inline int f2fs_sb_has_flexible_inline_xattr(struct super_block *sb); >> static inline int get_inline_xattr_addrs(struct inode *inode) >> { >> - if (!f2fs_has_inline_xattr(inode)) >> - return 0; >> - if (!f2fs_sb_has_flexible_inline_xattr(F2FS_I_SB(inode)->sb)) >> - return DEFAULT_INLINE_XATTR_ADDRS; >> + >> return F2FS_I(inode)->i_inline_xattr_size; >> } >> >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >> index bb876737e653..7f31b22c9efa 100644 >> --- a/fs/f2fs/inode.c >> +++ b/fs/f2fs/inode.c >> @@ -232,10 +232,19 @@ static int do_read_inode(struct inode *inode) >> fi->i_extra_isize = f2fs_has_extra_attr(inode) ? >> le16_to_cpu(ri->i_extra_isize) : 0; >> >> - if (!f2fs_has_inline_xattr(inode)) >> - fi->i_inline_xattr_size = 0; >> - else if (f2fs_sb_has_flexible_inline_xattr(sbi->sb)) >> + /* >> + * Previously, we will always reserve DEFAULT_INLINE_XATTR_ADDRS size >> + * space for inline xattr datas, if inline xattr is not enabled, we >> + * can expect all zero in reserved area, so for regular or symlink, >> + * it will be safe to reuse reserved area, but for directory, we >> + * should keep the reservation for stablizing directory structure. >> + */ >> + if (f2fs_has_extra_attr(inode) && >> + f2fs_sb_has_flexible_inline_xattr(sbi->sb)) >> fi->i_inline_xattr_size = le16_to_cpu(ri->i_inline_xattr_size); >> + else if (!f2fs_has_inline_xattr(inode) && >> + (S_ISREG(inode->i_mode) || S_ISLNK(inode->i_mode))) >> + fi->i_inline_xattr_size = 0; >> else >> fi->i_inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS; >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index e6f86d5d97b9..a1c56a14c191 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -91,6 +91,8 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode) >> f2fs_sb_has_flexible_inline_xattr(sbi->sb) && >> f2fs_has_inline_xattr(inode)) >> F2FS_I(inode)->i_inline_xattr_size = sbi->inline_xattr_size; >> + else >> + F2FS_I(inode)->i_inline_xattr_size = 0; >> >> if (test_opt(sbi, INLINE_DATA) && f2fs_may_inline_data(inode)) >> set_inode_flag(inode, FI_INLINE_DATA); >> -- >> 2.13.1.388.g69e6b9b4f4a9 > > . >