lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 29 Jul 2017 08:27:37 +0800 From: Chao Yu <yuchao0@...wei.com> To: <jaegeuk@...nel.org> CC: Chao Yu <chao@...nel.org>, <linux-f2fs-devel@...ts.sourceforge.net>, <linux-kernel@...r.kernel.org> Subject: Re: [RFC PATCH] f2fs: obsolete FI_ACL_MODE Hi Jaegeuk, Could you take time to have a look at this? Is this change reasonable? Thanks, On 2017/7/26 22:33, Chao Yu wrote: > From: Chao Yu <yuchao0@...wei.com> > > Previously, in order to avoid losing important inode metadata after > checkpoint & sudden power-off, f2fs uses synchronous approach for > updating inode metadata, so attribute in inode cache will be updated > to node page cache, after flushing dirty node pages in checkpoint, > these attribute in node page can be persisted. > > However, f2fs has changed to use asynchronous inode metadata update > approach in commit 0f18b462b2e5 ("flush inode metadata when checkpoint > is doing"), with this change, we can obsolete synchrounous metadata > update approach including old acl updating method.> > Signed-off-by: Chao Yu <yuchao0@...wei.com> > --- > fs/f2fs/acl.c | 8 +++----- > fs/f2fs/f2fs.h | 13 ------------- > fs/f2fs/file.c | 43 ++++--------------------------------------- > fs/f2fs/xattr.c | 6 +----- > 4 files changed, 8 insertions(+), 62 deletions(-) > > diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c > index b4b8438c42ef..25eaf243ee2e 100644 > --- a/fs/f2fs/acl.c > +++ b/fs/f2fs/acl.c > @@ -215,7 +215,8 @@ static int __f2fs_set_acl(struct inode *inode, int type, > error = posix_acl_update_mode(inode, &inode->i_mode, &acl); > if (error) > return error; > - set_acl_inode(inode, inode->i_mode); > + inode->i_ctime = current_time(inode); > + f2fs_mark_inode_dirty_sync(inode, true); > } > break; > > @@ -231,10 +232,8 @@ static int __f2fs_set_acl(struct inode *inode, int type, > > if (acl) { > value = f2fs_acl_to_disk(F2FS_I_SB(inode), acl, &size); > - if (IS_ERR(value)) { > - clear_inode_flag(inode, FI_ACL_MODE); > + if (IS_ERR(value)) > return PTR_ERR(value); > - } > } > > error = f2fs_setxattr(inode, name_index, "", value, size, ipage, 0); > @@ -243,7 +242,6 @@ static int __f2fs_set_acl(struct inode *inode, int type, > if (!error) > set_cached_acl(inode, type, acl); > > - clear_inode_flag(inode, FI_ACL_MODE); > return error; > } > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 5cfffd6f0209..07a6697a0589 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -561,7 +561,6 @@ struct f2fs_inode_info { > unsigned char i_dir_level; /* use for dentry level for large dir */ > unsigned int i_current_depth; /* use only in directory structure */ > unsigned int i_pino; /* parent inode number */ > - umode_t i_acl_mode; /* keep file acl mode temporarily */ > > /* Use below internally in f2fs*/ > unsigned long flags; /* use to pass per-file flags */ > @@ -1957,7 +1956,6 @@ enum { > FI_AUTO_RECOVER, /* indicate inode is recoverable */ > FI_DIRTY_DIR, /* indicate directory has dirty pages */ > FI_INC_LINK, /* need to increment i_nlink */ > - FI_ACL_MODE, /* indicate acl mode */ > FI_NO_ALLOC, /* should not allocate any blocks */ > FI_FREE_NID, /* free allocated nide */ > FI_NO_EXTENT, /* not to use the extent cache */ > @@ -2016,13 +2014,6 @@ static inline void clear_inode_flag(struct inode *inode, int flag) > __mark_inode_dirty_flag(inode, flag, false); > } > > -static inline void set_acl_inode(struct inode *inode, umode_t mode) > -{ > - F2FS_I(inode)->i_acl_mode = mode; > - set_inode_flag(inode, FI_ACL_MODE); > - f2fs_mark_inode_dirty_sync(inode, false); > -} > - > static inline void f2fs_i_links_write(struct inode *inode, bool inc) > { > if (inc) > @@ -2295,10 +2286,6 @@ static inline int get_extra_isize(struct inode *inode) > return F2FS_I(inode)->i_extra_isize / sizeof(__le32); > } > > -#define get_inode_mode(i) \ > - ((is_inode_flag_set(i, FI_ACL_MODE)) ? \ > - (F2FS_I(i)->i_acl_mode) : ((i)->i_mode)) > - > #define F2FS_TOTAL_EXTRA_ATTR_SIZE \ > (offsetof(struct f2fs_inode, i_extra_end) - \ > offsetof(struct f2fs_inode, i_extra_isize)) \ > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index e13f0ada1110..f8d3f4f928e7 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -763,36 +763,6 @@ int f2fs_getattr(const struct path *path, struct kstat *stat, > return 0; > } > > -#ifdef CONFIG_F2FS_FS_POSIX_ACL > -static void __setattr_copy(struct inode *inode, const struct iattr *attr) > -{ > - unsigned int ia_valid = attr->ia_valid; > - > - if (ia_valid & ATTR_UID) > - inode->i_uid = attr->ia_uid; > - if (ia_valid & ATTR_GID) > - inode->i_gid = attr->ia_gid; > - if (ia_valid & ATTR_ATIME) > - inode->i_atime = timespec_trunc(attr->ia_atime, > - inode->i_sb->s_time_gran); > - if (ia_valid & ATTR_MTIME) > - inode->i_mtime = timespec_trunc(attr->ia_mtime, > - inode->i_sb->s_time_gran); > - if (ia_valid & ATTR_CTIME) > - inode->i_ctime = timespec_trunc(attr->ia_ctime, > - inode->i_sb->s_time_gran); > - if (ia_valid & ATTR_MODE) { > - umode_t mode = attr->ia_mode; > - > - if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) > - mode &= ~S_ISGID; > - set_acl_inode(inode, mode); > - } > -} > -#else > -#define __setattr_copy setattr_copy > -#endif > - > int f2fs_setattr(struct dentry *dentry, struct iattr *attr) > { > struct inode *inode = d_inode(dentry); > @@ -854,18 +824,13 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr) > size_changed = true; > } > > - __setattr_copy(inode, attr); > + setattr_copy(inode, attr); > > - if (attr->ia_valid & ATTR_MODE) { > - err = posix_acl_chmod(inode, get_inode_mode(inode)); > - if (err || is_inode_flag_set(inode, FI_ACL_MODE)) { > - inode->i_mode = F2FS_I(inode)->i_acl_mode; > - clear_inode_flag(inode, FI_ACL_MODE); > - } > - } > + if (attr->ia_valid & ATTR_MODE) > + err = posix_acl_chmod(inode, inode->i_mode); > > /* file size may changed here */ > - f2fs_mark_inode_dirty_sync(inode, size_changed); > + f2fs_mark_inode_dirty_sync(inode, size_changed && !err); > > /* inode change will produce dirty node pages flushed by checkpoint */ > f2fs_balance_fs(F2FS_I_SB(inode), true); > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c > index ef3a3721ea6b..d0420870f79f 100644 > --- a/fs/f2fs/xattr.c > +++ b/fs/f2fs/xattr.c > @@ -654,14 +654,10 @@ static int __f2fs_setxattr(struct inode *inode, int index, > if (error) > goto exit; > > - if (is_inode_flag_set(inode, FI_ACL_MODE)) { > - inode->i_mode = F2FS_I(inode)->i_acl_mode; > - inode->i_ctime = current_time(inode); > - clear_inode_flag(inode, FI_ACL_MODE); > - } > if (index == F2FS_XATTR_INDEX_ENCRYPTION && > !strcmp(name, F2FS_XATTR_NAME_ENCRYPTION_CONTEXT)) > f2fs_set_encrypted_inode(inode); > + inode->i_ctime = current_time(inode); > f2fs_mark_inode_dirty_sync(inode, true); > if (!error && S_ISDIR(inode->i_mode)) > set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); >
Powered by blists - more mailing lists