From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751346AbeACHfE (ORCPT + 1 other); Wed, 3 Jan 2018 02:35:04 -0500 Received: from szxga04-in.huawei.com ([45.249.212.190]:3666 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750749AbeACHfD (ORCPT ); Wed, 3 Jan 2018 02:35:03 -0500 Subject: Re: [PATCH v3] f2fs: add an ioctl to disable GC for specific file To: Jaegeuk Kim , , References: <20171208051555.76998-1-jaegeuk@kernel.org> <20171208193723.GA87676@jaegeuk-macbookpro.roam.corp.google.com> <20171228034020.GF13490@jaegeuk-macbookpro.roam.corp.google.com> <20180103032151.GC3995@jaegeuk-macbookpro.roam.corp.google.com> From: Chao Yu Message-ID: <0bac2cb3-5585-bfff-9de4-3b6837003347@huawei.com> Date: Wed, 3 Jan 2018 15:34:45 +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: <20180103032151.GC3995@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 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 2018/1/3 11:21, Jaegeuk Kim wrote: > This patch gives a flag to disable GC on given file, which would be useful, when > user wants to keep its block map. It also conducts in-place-update for dontmove > file. > > Signed-off-by: Jaegeuk Kim > --- > > Change log from v2: > - modify ioctl to allow users unpin the file > > fs/f2fs/data.c | 2 ++ > fs/f2fs/f2fs.h | 28 +++++++++++++++++++++- > fs/f2fs/file.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ > fs/f2fs/gc.c | 11 +++++++++ > fs/f2fs/gc.h | 2 ++ > fs/f2fs/sysfs.c | 2 ++ > include/linux/f2fs_fs.h | 9 ++++++- > 7 files changed, 116 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 449b0aaa3905..45f65a5b9871 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1395,6 +1395,8 @@ static inline bool need_inplace_update(struct f2fs_io_info *fio) > { > struct inode *inode = fio->page->mapping->host; > > + if (f2fs_is_pinned_file(inode)) > + return true; > if (S_ISDIR(inode->i_mode) || f2fs_is_atomic_file(inode)) > return false; > if (is_cold_data(fio->page)) > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index a0e8eec23125..f4b7d73695a7 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -350,6 +350,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal, > #define F2FS_IOC_GARBAGE_COLLECT_RANGE _IOW(F2FS_IOCTL_MAGIC, 11, \ > struct f2fs_gc_range) > #define F2FS_IOC_GET_FEATURES _IOR(F2FS_IOCTL_MAGIC, 12, __u32) > +#define F2FS_IOC_SET_PIN_FILE _IOW(F2FS_IOCTL_MAGIC, 13, __u32) > > #define F2FS_IOC_SET_ENCRYPTION_POLICY FS_IOC_SET_ENCRYPTION_POLICY > #define F2FS_IOC_GET_ENCRYPTION_POLICY FS_IOC_GET_ENCRYPTION_POLICY > @@ -587,7 +588,10 @@ struct f2fs_inode_info { > unsigned long i_flags; /* keep an inode flags for ioctl */ > unsigned char i_advise; /* use to give file attribute hints */ > unsigned char i_dir_level; /* use for dentry level for large dir */ > - unsigned int i_current_depth; /* use only in directory structure */ > + union { > + unsigned int i_current_depth; /* only for directory depth */ > + unsigned short i_gc_failures; /* only for regular file */ > + }; > unsigned int i_pino; /* parent inode number */ > umode_t i_acl_mode; /* keep file acl mode temporarily */ > > @@ -1133,6 +1137,9 @@ struct f2fs_sb_info { > /* threshold for converting bg victims for fg */ > u64 fggc_threshold; > > + /* threshold for gc trials on pinned files */ > + u64 gc_pin_file_threshold; > + > /* maximum # of trials to find a victim segment for SSR and GC */ > unsigned int max_victim_search; > > @@ -2124,6 +2131,7 @@ enum { > FI_HOT_DATA, /* indicate file is hot */ > FI_EXTRA_ATTR, /* indicate file has extra attribute */ > FI_PROJ_INHERIT, /* indicate file inherits projectid */ > + FI_PIN_FILE, /* indicate file should not be gced */ > }; > > static inline void __mark_inode_dirty_flag(struct inode *inode, > @@ -2137,6 +2145,7 @@ static inline void __mark_inode_dirty_flag(struct inode *inode, > return; > case FI_DATA_EXIST: > case FI_INLINE_DOTS: > + case FI_PIN_FILE: > f2fs_mark_inode_dirty_sync(inode, true); > } > } > @@ -2217,6 +2226,13 @@ static inline void f2fs_i_depth_write(struct inode *inode, unsigned int depth) > f2fs_mark_inode_dirty_sync(inode, true); > } > > +static inline void f2fs_i_gc_failures_write(struct inode *inode, > + unsigned int count) > +{ > + F2FS_I(inode)->i_gc_failures = count; > + f2fs_mark_inode_dirty_sync(inode, true); > +} > + > static inline void f2fs_i_xnid_write(struct inode *inode, nid_t xnid) > { > F2FS_I(inode)->i_xattr_nid = xnid; > @@ -2245,6 +2261,8 @@ static inline void get_inline_info(struct inode *inode, struct f2fs_inode *ri) > set_bit(FI_INLINE_DOTS, &fi->flags); > if (ri->i_inline & F2FS_EXTRA_ATTR) > set_bit(FI_EXTRA_ATTR, &fi->flags); > + if (ri->i_inline & F2FS_PIN_FILE) > + set_bit(FI_PIN_FILE, &fi->flags); > } > > static inline void set_raw_inline(struct inode *inode, struct f2fs_inode *ri) > @@ -2263,6 +2281,8 @@ static inline void set_raw_inline(struct inode *inode, struct f2fs_inode *ri) > ri->i_inline |= F2FS_INLINE_DOTS; > if (is_inode_flag_set(inode, FI_EXTRA_ATTR)) > ri->i_inline |= F2FS_EXTRA_ATTR; > + if (is_inode_flag_set(inode, FI_PIN_FILE)) > + ri->i_inline |= F2FS_PIN_FILE; > } > > static inline int f2fs_has_extra_attr(struct inode *inode) > @@ -2308,6 +2328,11 @@ static inline int f2fs_has_inline_dots(struct inode *inode) > return is_inode_flag_set(inode, FI_INLINE_DOTS); > } > > +static inline bool f2fs_is_pinned_file(struct inode *inode) > +{ > + return is_inode_flag_set(inode, FI_PIN_FILE); > +} > + > static inline bool f2fs_is_atomic_file(struct inode *inode) > { > return is_inode_flag_set(inode, FI_ATOMIC_FILE); > @@ -2523,6 +2548,7 @@ int truncate_hole(struct inode *inode, pgoff_t pg_start, pgoff_t pg_end); > void truncate_data_blocks_range(struct dnode_of_data *dn, int count); > long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); > long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg); > +int f2fs_pin_file_control(struct inode *inode, bool inc); > > /* > * inode.c > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 322aab9d91b5..a7ee659c5a61 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -2669,6 +2669,67 @@ static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg) > return 0; > } > > +int f2fs_pin_file_control(struct inode *inode, bool inc) > +{ > + struct f2fs_inode_info *fi = F2FS_I(inode); > + struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > + > + /* Use i_gc_failures for normal file as a risk signal. */ > + if (inc) > + f2fs_i_gc_failures_write(inode, fi->i_gc_failures + 1); > + > + if (fi->i_gc_failures > sbi->gc_pin_file_threshold) { > + f2fs_msg(sbi->sb, KERN_WARNING, > + "%s: Enable GC = ino %lx after %x GC trials\n", > + __func__, inode->i_ino, fi->i_gc_failures); > + return -EAGAIN; > + } > + return 0; > +} > + > +static int f2fs_ioc_set_pin_file(struct file *filp, unsigned long arg) > +{ > + struct inode *inode = file_inode(filp); > + __u32 pin; > + int ret = 0; > + > + if (!inode_owner_or_capable(inode)) > + return -EACCES; > + > + if (get_user(pin, (__u32 __user *)arg)) > + return -EFAULT; > + > + if (!S_ISREG(inode->i_mode)) > + return -EINVAL; > + > + if (f2fs_readonly(F2FS_I_SB(inode)->sb)) > + return -EROFS; mnt_want_write_file(filp)? > + > + inode_lock(inode); > + > + if (!pin) { > + clear_inode_flag(inode, FI_PIN_FILE); > + F2FS_I(inode)->i_gc_failures = 0; > + goto done; > + } > + > + if (f2fs_pin_file_control(inode, false)) { > + ret = -EAGAIN; > + goto out; > + } > + ret = f2fs_convert_inline_inode(inode); > + if (ret) > + goto out; > + > + set_inode_flag(inode, FI_PIN_FILE); > + ret = F2FS_I(inode)->i_gc_failures; > +done: > + f2fs_update_time(F2FS_I_SB(inode), REQ_TIME); > +out: > + inode_unlock(inode); mnt_drop_write_file(filp)? Other part looks good to me. ;) Reviewed-by: Chao Yu Thanks, > + return ret; > +} > + > long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > { > if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp))))) > @@ -2719,6 +2780,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > return f2fs_ioc_fsgetxattr(filp, arg); > case F2FS_IOC_FSSETXATTR: > return f2fs_ioc_fssetxattr(filp, arg); > + case F2FS_IOC_SET_PIN_FILE: > + return f2fs_ioc_set_pin_file(filp, arg); > default: > return -ENOTTY; > } > @@ -2794,6 +2857,7 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > case F2FS_IOC_GET_FEATURES: > case F2FS_IOC_FSGETXATTR: > case F2FS_IOC_FSSETXATTR: > + case F2FS_IOC_SET_PIN_FILE: > break; > default: > return -ENOIOCTLCMD; > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 5d5bba462f26..9bffef153a12 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -624,6 +624,11 @@ static void move_data_block(struct inode *inode, block_t bidx, > if (f2fs_is_atomic_file(inode)) > goto out; > > + if (f2fs_is_pinned_file(inode)) { > + f2fs_pin_file_control(inode, true); > + goto out; > + } > + > set_new_dnode(&dn, inode, NULL, NULL, 0); > err = get_dnode_of_data(&dn, bidx, LOOKUP_NODE); > if (err) > @@ -720,6 +725,11 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type, > > if (f2fs_is_atomic_file(inode)) > goto out; > + if (f2fs_is_pinned_file(inode)) { > + if (gc_type == FG_GC) > + f2fs_pin_file_control(inode, true); > + goto out; > + } > > if (gc_type == BG_GC) { > if (PageWriteback(page)) > @@ -1091,6 +1101,7 @@ void build_gc_manager(struct f2fs_sb_info *sbi) > > sbi->fggc_threshold = div64_u64((main_count - ovp_count) * > BLKS_PER_SEC(sbi), (main_count - resv_count)); > + sbi->gc_pin_file_threshold = DEF_GC_FAILED_PINNED_FILES; > > /* give warm/cold data area from slower device */ > if (sbi->s_ndevs && sbi->segs_per_sec == 1) > diff --git a/fs/f2fs/gc.h b/fs/f2fs/gc.h > index 9325191fab2d..33310165cb78 100644 > --- a/fs/f2fs/gc.h > +++ b/fs/f2fs/gc.h > @@ -20,6 +20,8 @@ > #define LIMIT_INVALID_BLOCK 40 /* percentage over total user space */ > #define LIMIT_FREE_BLOCK 40 /* percentage over invalid + free space */ > > +#define DEF_GC_FAILED_PINNED_FILES 1024 > + > /* Search max. number of dirty segments to select a victim segment */ > #define DEF_MAX_VICTIM_SEARCH 4096 /* covers 8GB */ > > diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c > index ab6028c332aa..41887e6ec1b3 100644 > --- a/fs/f2fs/sysfs.c > +++ b/fs/f2fs/sysfs.c > @@ -301,6 +301,7 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, cp_interval, interval_time[CP_TIME]); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, idle_interval, interval_time[REQ_TIME]); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, iostat_enable, iostat_enable); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, readdir_ra, readdir_ra); > +F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_pin_file_thresh, gc_pin_file_threshold); > #ifdef CONFIG_F2FS_FAULT_INJECTION > F2FS_RW_ATTR(FAULT_INFO_RATE, f2fs_fault_info, inject_rate, inject_rate); > F2FS_RW_ATTR(FAULT_INFO_TYPE, f2fs_fault_info, inject_type, inject_type); > @@ -349,6 +350,7 @@ static struct attribute *f2fs_attrs[] = { > ATTR_LIST(idle_interval), > ATTR_LIST(iostat_enable), > ATTR_LIST(readdir_ra), > + ATTR_LIST(gc_pin_file_thresh), > #ifdef CONFIG_F2FS_FAULT_INJECTION > ATTR_LIST(inject_rate), > ATTR_LIST(inject_type), > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h > index 43e98d30d2df..cfb522e6affc 100644 > --- a/include/linux/f2fs_fs.h > +++ b/include/linux/f2fs_fs.h > @@ -212,6 +212,7 @@ struct f2fs_extent { > #define F2FS_DATA_EXIST 0x08 /* file inline data exist flag */ > #define F2FS_INLINE_DOTS 0x10 /* file having implicit dot dentries */ > #define F2FS_EXTRA_ATTR 0x20 /* file having extra attribute */ > +#define F2FS_PIN_FILE 0x40 /* file should not be gced */ > > struct f2fs_inode { > __le16 i_mode; /* file mode */ > @@ -229,7 +230,13 @@ struct f2fs_inode { > __le32 i_ctime_nsec; /* change time in nano scale */ > __le32 i_mtime_nsec; /* modification time in nano scale */ > __le32 i_generation; /* file version (for NFS) */ > - __le32 i_current_depth; /* only for directory depth */ > + union { > + __le32 i_current_depth; /* only for directory depth */ > + __le16 i_gc_failures; /* > + * # of gc failures on pinned file. > + * only for regular files. > + */ > + }; > __le32 i_xattr_nid; /* nid to save xattr */ > __le32 i_flags; /* file attributes */ > __le32 i_pino; /* parent inode number */ >