From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932228AbdJZKhe (ORCPT ); Thu, 26 Oct 2017 06:37:34 -0400 Received: from mail.kernel.org ([198.145.29.99]:51770 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751251AbdJZKhc (ORCPT ); Thu, 26 Oct 2017 06:37:32 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3697B21913 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jaegeuk@kernel.org Date: Thu, 26 Oct 2017 12:37:28 +0200 From: Jaegeuk Kim To: Chao Yu Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, chao@kernel.org Subject: Re: [PATCH] f2fs: fix to keep backward compatibility of flexible inline xattr feature Message-ID: <20171026103728.GA62112@jaegeuk-macbookpro.roam.corp.google.com> References: <20171025103124.63162-1-yuchao0@huawei.com> <20171026084243.GA54331@jaegeuk-macbookpro.roam.corp.google.com> <37f397bd-fa5b-c7f0-bb20-ea3be98437cd@huawei.com> <20171026091108.GC54331@jaegeuk-macbookpro.roam.corp.google.com> <20171026100257.GA60038@jaegeuk-macbookpro.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171026100257.GA60038@jaegeuk-macbookpro.roam.corp.google.com> User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/26, Jaegeuk Kim wrote: > Hi Chao, > > On 10/26, Jaegeuk Kim wrote: > > On 10/26, Chao Yu wrote: > > > 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". > > BTW, I read the patch again, and couldn't catch the problem actually. > We didn't assign inline_xattr all the time, instead do if inline_xattr > is set. Have you done some tests with this? I'm failing tests with these > changes. Could you take a look at this change? --- fs/f2fs/f2fs.h | 1 - fs/f2fs/inode.c | 20 ++++++-------------- fs/f2fs/namei.c | 16 ++++++++++------ 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index f7fb295..97358d7 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2373,7 +2373,6 @@ 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) { - return F2FS_I(inode)->i_inline_xattr_size; } diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index cef2f65..eb9a21e 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -233,21 +233,14 @@ 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; - /* - * 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)) + if (f2fs_sb_has_flexible_inline_xattr(sbi->sb)) { + f2fs_bug_on(sbi, !f2fs_has_extra_attr(inode)); 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))) + } else if (!f2fs_has_inline_xattr(inode)) { fi->i_inline_xattr_size = 0; - else + } else { fi->i_inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS; + } /* check data exist */ if (f2fs_has_inline_data(inode) && !f2fs_exist_data(inode)) @@ -401,8 +394,7 @@ int update_inode(struct inode *inode, struct page *node_page) if (f2fs_has_extra_attr(inode)) { ri->i_extra_isize = cpu_to_le16(F2FS_I(inode)->i_extra_isize); - if (f2fs_sb_has_flexible_inline_xattr(F2FS_I_SB(inode)->sb) && - f2fs_has_inline_xattr(inode)) + if (f2fs_sb_has_flexible_inline_xattr(F2FS_I_SB(inode)->sb)) ri->i_inline_xattr_size = cpu_to_le16(F2FS_I(inode)->i_inline_xattr_size); diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index e8fdd5e..7ec86d3 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -29,6 +29,7 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode) nid_t ino; struct inode *inode; bool nid_free = false; + int xattr_size = 0; int err; inode = new_inode(dir->i_sb); @@ -87,12 +88,15 @@ static struct inode *f2fs_new_inode(struct inode *dir, umode_t mode) if (test_opt(sbi, INLINE_XATTR)) set_inode_flag(inode, FI_INLINE_XATTR); - if (f2fs_sb_has_extra_attr(sbi->sb) && - 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 (f2fs_has_inline_xattr(inode)) { + if (f2fs_sb_has_flexible_inline_xattr(sbi->sb)) { + f2fs_bug_on(sbi, !f2fs_has_extra_attr(inode)); + xattr_size = sbi->inline_xattr_size; + } else { + xattr_size = DEFAULT_INLINE_XATTR_ADDRS; + } + } + F2FS_I(inode)->i_inline_xattr_size = xattr_size; if (test_opt(sbi, INLINE_DATA) && f2fs_may_inline_data(inode)) set_inode_flag(inode, FI_INLINE_DATA); -- 2.7.4