* [PATCH] f2fs: remain written times to update inode during fsync @ 2018-03-30 5:51 Jaegeuk Kim 2018-03-30 10:54 ` Chao Yu 2018-03-30 16:30 ` [PATCH v2] " Jaegeuk Kim 0 siblings, 2 replies; 10+ messages in thread From: Jaegeuk Kim @ 2018-03-30 5:51 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim This fixes xfstests/generic/392. Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/f2fs.h | 15 +++++++++++++++ fs/f2fs/inode.c | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 000f93f6767e..675c39d85111 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -664,6 +664,7 @@ struct f2fs_inode_info { kprojid_t i_projid; /* id for project quota */ int i_inline_xattr_size; /* inline xattr size */ struct timespec i_crtime; /* inode creation time */ + struct timespec i_disk_time[4]; /* inode disk times */ }; static inline void get_extent_info(struct extent_info *ext, @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) f2fs_mark_inode_dirty_sync(inode, true); } +static inline bool time_equal(struct timespec a, struct timespec b) +{ + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); +} + static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) { bool ret; @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) i_size_read(inode) & ~PAGE_MASK) return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) + return false; + down_read(&F2FS_I(inode)->i_sem); ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); up_read(&F2FS_I(inode)->i_sem); diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 401f09ccce7e..70aba580f4b0 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) if (inode->i_nlink == 0) clear_inline_node(node_page); + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; } void update_inode_page(struct inode *inode) -- 2.15.0.531.g2ccb3012c9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: remain written times to update inode during fsync 2018-03-30 5:51 [PATCH] f2fs: remain written times to update inode during fsync Jaegeuk Kim @ 2018-03-30 10:54 ` Chao Yu 2018-03-30 16:31 ` Jaegeuk Kim 2018-03-30 16:30 ` [PATCH v2] " Jaegeuk Kim 1 sibling, 1 reply; 10+ messages in thread From: Chao Yu @ 2018-03-30 10:54 UTC (permalink / raw) To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel Hi Jaegeuk, On 2018/3/30 13:51, Jaegeuk Kim wrote: > This fixes xfstests/generic/392. Hmm... Could you please give more details about this issue and solution in commit message, since I can catch up the solution only with the code. Thanks, > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > fs/f2fs/f2fs.h | 15 +++++++++++++++ > fs/f2fs/inode.c | 4 ++++ > 2 files changed, 19 insertions(+) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 000f93f6767e..675c39d85111 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -664,6 +664,7 @@ struct f2fs_inode_info { > kprojid_t i_projid; /* id for project quota */ > int i_inline_xattr_size; /* inline xattr size */ > struct timespec i_crtime; /* inode creation time */ > + struct timespec i_disk_time[4]; /* inode disk times */ > }; > > static inline void get_extent_info(struct extent_info *ext, > @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) > f2fs_mark_inode_dirty_sync(inode, true); > } > > +static inline bool time_equal(struct timespec a, struct timespec b) > +{ > + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); > +} > + > static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > { > bool ret; > @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > i_size_read(inode) & ~PAGE_MASK) > return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) > + return false; > + > down_read(&F2FS_I(inode)->i_sem); > ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); > up_read(&F2FS_I(inode)->i_sem); > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 401f09ccce7e..70aba580f4b0 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) > if (inode->i_nlink == 0) > clear_inline_node(node_page); > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; > } > > void update_inode_page(struct inode *inode) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] f2fs: remain written times to update inode during fsync 2018-03-30 10:54 ` Chao Yu @ 2018-03-30 16:31 ` Jaegeuk Kim 0 siblings, 0 replies; 10+ messages in thread From: Jaegeuk Kim @ 2018-03-30 16:31 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 03/30, Chao Yu wrote: > Hi Jaegeuk, > > On 2018/3/30 13:51, Jaegeuk Kim wrote: > > This fixes xfstests/generic/392. > > Hmm... Could you please give more details about this issue and solution in > commit message, since I can catch up the solution only with the code. Yeah, I missed some explanation. :P > > Thanks, > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/f2fs.h | 15 +++++++++++++++ > > fs/f2fs/inode.c | 4 ++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 000f93f6767e..675c39d85111 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -664,6 +664,7 @@ struct f2fs_inode_info { > > kprojid_t i_projid; /* id for project quota */ > > int i_inline_xattr_size; /* inline xattr size */ > > struct timespec i_crtime; /* inode creation time */ > > + struct timespec i_disk_time[4]; /* inode disk times */ > > }; > > > > static inline void get_extent_info(struct extent_info *ext, > > @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) > > f2fs_mark_inode_dirty_sync(inode, true); > > } > > > > +static inline bool time_equal(struct timespec a, struct timespec b) > > +{ > > + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); > > +} > > + > > static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > > { > > bool ret; > > @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > > i_size_read(inode) & ~PAGE_MASK) > > return false; > > > > + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) > > + return false; > > + > > down_read(&F2FS_I(inode)->i_sem); > > ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); > > up_read(&F2FS_I(inode)->i_sem); > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > > index 401f09ccce7e..70aba580f4b0 100644 > > --- a/fs/f2fs/inode.c > > +++ b/fs/f2fs/inode.c > > @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) > > if (inode->i_nlink == 0) > > clear_inline_node(node_page); > > > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; > > } > > > > void update_inode_page(struct inode *inode) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-03-30 5:51 [PATCH] f2fs: remain written times to update inode during fsync Jaegeuk Kim 2018-03-30 10:54 ` Chao Yu @ 2018-03-30 16:30 ` Jaegeuk Kim 2018-04-03 2:29 ` Chao Yu 2018-04-03 5:22 ` [f2fs-dev] [PATCH v3] " Jaegeuk Kim 1 sibling, 2 replies; 10+ messages in thread From: Jaegeuk Kim @ 2018-03-30 16:30 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel Change log from v1: - add more description This fixes xfstests/generic/392. The failure was caused by different times between 1) one marked in the last fsync(2) call and 2) the other given by roll-forward recovery after power-cut. The reason was that we skipped updating inode block at 1), since its i_size was recoverable along with 4KB-aligned data writes, which was fixed by: "f2fs: fix a wrong condition in f2fs_skip_inode_update" Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/f2fs.h | 15 +++++++++++++++ fs/f2fs/inode.c | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 000f93f6767e..675c39d85111 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -664,6 +664,7 @@ struct f2fs_inode_info { kprojid_t i_projid; /* id for project quota */ int i_inline_xattr_size; /* inline xattr size */ struct timespec i_crtime; /* inode creation time */ + struct timespec i_disk_time[4]; /* inode disk times */ }; static inline void get_extent_info(struct extent_info *ext, @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) f2fs_mark_inode_dirty_sync(inode, true); } +static inline bool time_equal(struct timespec a, struct timespec b) +{ + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); +} + static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) { bool ret; @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) i_size_read(inode) & ~PAGE_MASK) return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) + return false; + down_read(&F2FS_I(inode)->i_sem); ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); up_read(&F2FS_I(inode)->i_sem); diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 401f09ccce7e..70aba580f4b0 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) if (inode->i_nlink == 0) clear_inline_node(node_page); + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; } void update_inode_page(struct inode *inode) -- 2.15.0.531.g2ccb3012c9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-03-30 16:30 ` [PATCH v2] " Jaegeuk Kim @ 2018-04-03 2:29 ` Chao Yu 2018-04-03 5:23 ` Jaegeuk Kim 2018-04-03 5:22 ` [f2fs-dev] [PATCH v3] " Jaegeuk Kim 1 sibling, 1 reply; 10+ messages in thread From: Chao Yu @ 2018-04-03 2:29 UTC (permalink / raw) To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel On 2018/3/31 0:30, Jaegeuk Kim wrote: > Change log from v1: > - add more description > > This fixes xfstests/generic/392. > > The failure was caused by different times between 1) one marked in the last > fsync(2) call and 2) the other given by roll-forward recovery after power-cut. > The reason was that we skipped updating inode block at 1), since its i_size > was recoverable along with 4KB-aligned data writes, which was fixed by: > "f2fs: fix a wrong condition in f2fs_skip_inode_update" > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > fs/f2fs/f2fs.h | 15 +++++++++++++++ > fs/f2fs/inode.c | 4 ++++ > 2 files changed, 19 insertions(+) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 000f93f6767e..675c39d85111 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -664,6 +664,7 @@ struct f2fs_inode_info { > kprojid_t i_projid; /* id for project quota */ > int i_inline_xattr_size; /* inline xattr size */ > struct timespec i_crtime; /* inode creation time */ > + struct timespec i_disk_time[4]; /* inode disk times */ > }; > > static inline void get_extent_info(struct extent_info *ext, > @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) > f2fs_mark_inode_dirty_sync(inode, true); > } > > +static inline bool time_equal(struct timespec a, struct timespec b) > +{ > + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); > +} We can use existing timespec_equal in <linux/time.h> instead of defining a duplicated one? > + > static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > { > bool ret; > @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > i_size_read(inode) & ~PAGE_MASK) > return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) > + return false; > + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) > + return false; > + > down_read(&F2FS_I(inode)->i_sem); > ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); > up_read(&F2FS_I(inode)->i_sem); > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 401f09ccce7e..70aba580f4b0 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) > if (inode->i_nlink == 0) > clear_inline_node(node_page); > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; We need initialize them in do_read_inode? Thanks, > } > > void update_inode_page(struct inode *inode) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-04-03 2:29 ` Chao Yu @ 2018-04-03 5:23 ` Jaegeuk Kim 2018-04-03 7:17 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2018-04-03 5:23 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 04/03, Chao Yu wrote: > On 2018/3/31 0:30, Jaegeuk Kim wrote: > > Change log from v1: > > - add more description > > > > This fixes xfstests/generic/392. > > > > The failure was caused by different times between 1) one marked in the last > > fsync(2) call and 2) the other given by roll-forward recovery after power-cut. > > The reason was that we skipped updating inode block at 1), since its i_size > > was recoverable along with 4KB-aligned data writes, which was fixed by: > > "f2fs: fix a wrong condition in f2fs_skip_inode_update" > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/f2fs.h | 15 +++++++++++++++ > > fs/f2fs/inode.c | 4 ++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 000f93f6767e..675c39d85111 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -664,6 +664,7 @@ struct f2fs_inode_info { > > kprojid_t i_projid; /* id for project quota */ > > int i_inline_xattr_size; /* inline xattr size */ > > struct timespec i_crtime; /* inode creation time */ > > + struct timespec i_disk_time[4]; /* inode disk times */ > > }; > > > > static inline void get_extent_info(struct extent_info *ext, > > @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) > > f2fs_mark_inode_dirty_sync(inode, true); > > } > > > > +static inline bool time_equal(struct timespec a, struct timespec b) > > +{ > > + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); > > +} > > We can use existing timespec_equal in <linux/time.h> instead of defining a > duplicated one? It is defined with const parameters. > > > + > > static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > > { > > bool ret; > > @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > > i_size_read(inode) & ~PAGE_MASK) > > return false; > > > > + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) > > + return false; > > + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) > > + return false; > > + > > down_read(&F2FS_I(inode)->i_sem); > > ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); > > up_read(&F2FS_I(inode)->i_sem); > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > > index 401f09ccce7e..70aba580f4b0 100644 > > --- a/fs/f2fs/inode.c > > +++ b/fs/f2fs/inode.c > > @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) > > if (inode->i_nlink == 0) > > clear_inline_node(node_page); > > > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; > > We need initialize them in do_read_inode? Done. Thanks, > > Thanks, > > > } > > > > void update_inode_page(struct inode *inode) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-04-03 5:23 ` Jaegeuk Kim @ 2018-04-03 7:17 ` Chao Yu 2018-04-03 16:08 ` Jaegeuk Kim 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2018-04-03 7:17 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2018/4/3 13:23, Jaegeuk Kim wrote: > On 04/03, Chao Yu wrote: >> On 2018/3/31 0:30, Jaegeuk Kim wrote: >>> Change log from v1: >>> - add more description >>> >>> This fixes xfstests/generic/392. >>> >>> The failure was caused by different times between 1) one marked in the last >>> fsync(2) call and 2) the other given by roll-forward recovery after power-cut. >>> The reason was that we skipped updating inode block at 1), since its i_size >>> was recoverable along with 4KB-aligned data writes, which was fixed by: >>> "f2fs: fix a wrong condition in f2fs_skip_inode_update" >>> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>> --- >>> fs/f2fs/f2fs.h | 15 +++++++++++++++ >>> fs/f2fs/inode.c | 4 ++++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>> index 000f93f6767e..675c39d85111 100644 >>> --- a/fs/f2fs/f2fs.h >>> +++ b/fs/f2fs/f2fs.h >>> @@ -664,6 +664,7 @@ struct f2fs_inode_info { >>> kprojid_t i_projid; /* id for project quota */ >>> int i_inline_xattr_size; /* inline xattr size */ >>> struct timespec i_crtime; /* inode creation time */ >>> + struct timespec i_disk_time[4]; /* inode disk times */ >>> }; >>> >>> static inline void get_extent_info(struct extent_info *ext, >>> @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) >>> f2fs_mark_inode_dirty_sync(inode, true); >>> } >>> >>> +static inline bool time_equal(struct timespec a, struct timespec b) >>> +{ >>> + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); >>> +} >> >> We can use existing timespec_equal in <linux/time.h> instead of defining a >> duplicated one? > > It is defined with const parameters. I didn't get it, const keyword can used to protect parameter not to be changed in that function, that will be more safe, so it will be better to use that one? Thanks, > >> >>> + >>> static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) >>> { >>> bool ret; >>> @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) >>> i_size_read(inode) & ~PAGE_MASK) >>> return false; >>> >>> + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) >>> + return false; >>> + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) >>> + return false; >>> + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) >>> + return false; >>> + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) >>> + return false; >>> + >>> down_read(&F2FS_I(inode)->i_sem); >>> ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); >>> up_read(&F2FS_I(inode)->i_sem); >>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >>> index 401f09ccce7e..70aba580f4b0 100644 >>> --- a/fs/f2fs/inode.c >>> +++ b/fs/f2fs/inode.c >>> @@ -444,6 +444,10 @@ void update_inode(struct inode *inode, struct page *node_page) >>> if (inode->i_nlink == 0) >>> clear_inline_node(node_page); >>> >>> + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; >>> + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; >>> + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; >>> + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; >> >> We need initialize them in do_read_inode? > > Done. > Thanks, > >> >> Thanks, >> >>> } >>> >>> void update_inode_page(struct inode *inode) >>> > > . > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-04-03 7:17 ` Chao Yu @ 2018-04-03 16:08 ` Jaegeuk Kim 2018-04-04 1:27 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2018-04-03 16:08 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 04/03, Chao Yu wrote: > On 2018/4/3 13:23, Jaegeuk Kim wrote: > > On 04/03, Chao Yu wrote: > >> On 2018/3/31 0:30, Jaegeuk Kim wrote: > >>> Change log from v1: > >>> - add more description > >>> > >>> This fixes xfstests/generic/392. > >>> > >>> The failure was caused by different times between 1) one marked in the last > >>> fsync(2) call and 2) the other given by roll-forward recovery after power-cut. > >>> The reason was that we skipped updating inode block at 1), since its i_size > >>> was recoverable along with 4KB-aligned data writes, which was fixed by: > >>> "f2fs: fix a wrong condition in f2fs_skip_inode_update" > >>> > >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > >>> --- > >>> fs/f2fs/f2fs.h | 15 +++++++++++++++ > >>> fs/f2fs/inode.c | 4 ++++ > >>> 2 files changed, 19 insertions(+) > >>> > >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>> index 000f93f6767e..675c39d85111 100644 > >>> --- a/fs/f2fs/f2fs.h > >>> +++ b/fs/f2fs/f2fs.h > >>> @@ -664,6 +664,7 @@ struct f2fs_inode_info { > >>> kprojid_t i_projid; /* id for project quota */ > >>> int i_inline_xattr_size; /* inline xattr size */ > >>> struct timespec i_crtime; /* inode creation time */ > >>> + struct timespec i_disk_time[4]; /* inode disk times */ > >>> }; > >>> > >>> static inline void get_extent_info(struct extent_info *ext, > >>> @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) > >>> f2fs_mark_inode_dirty_sync(inode, true); > >>> } > >>> > >>> +static inline bool time_equal(struct timespec a, struct timespec b) > >>> +{ > >>> + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); > >>> +} > >> > >> We can use existing timespec_equal in <linux/time.h> instead of defining a > >> duplicated one? > > > > It is defined with const parameters. > > I didn't get it, const keyword can used to protect parameter not to be changed > in that function, that will be more safe, so it will be better to use that one? Oh, I just made a mistake when testing timespec_equal(). I really need more recess time. :P Change log from v3: - use timespec_equal() This fixes xfstests/generic/392. The failure was caused by different times between 1) one marked in the last fsync(2) call and 2) the other given by roll-forward recovery after power-cut. The reason was that we skipped updating inode block at 1), since its i_size was recoverable along with 4KB-aligned data writes, which was fixed by: "f2fs: fix a wrong condition in f2fs_skip_inode_update" Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/f2fs.h | 11 +++++++++++ fs/f2fs/inode.c | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 7102d3965fea..1df7f10476d6 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -664,6 +664,7 @@ struct f2fs_inode_info { kprojid_t i_projid; /* id for project quota */ int i_inline_xattr_size; /* inline xattr size */ struct timespec i_crtime; /* inode creation time */ + struct timespec i_disk_time[4]; /* inode disk times */ }; static inline void get_extent_info(struct extent_info *ext, @@ -2474,6 +2475,16 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) i_size_read(inode) & ~PAGE_MASK) return false; + if (!timespec_equal(F2FS_I(inode)->i_disk_time, &inode->i_atime)) + return false; + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 1, &inode->i_ctime)) + return false; + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 2, &inode->i_mtime)) + return false; + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 3, + &F2FS_I(inode)->i_crtime)) + return false; + down_read(&F2FS_I(inode)->i_sem); ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); up_read(&F2FS_I(inode)->i_sem); diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 401f09ccce7e..e0d9e8f27ed2 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -284,6 +284,10 @@ static int do_read_inode(struct inode *inode) fi->i_crtime.tv_nsec = le32_to_cpu(ri->i_crtime_nsec); } + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; f2fs_put_page(node_page, 1); stat_inc_inline_xattr(inode); @@ -444,6 +448,10 @@ void update_inode(struct inode *inode, struct page *node_page) if (inode->i_nlink == 0) clear_inline_node(node_page); + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; } void update_inode_page(struct inode *inode) -- 2.15.0.531.g2ccb3012c9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] f2fs: remain written times to update inode during fsync 2018-04-03 16:08 ` Jaegeuk Kim @ 2018-04-04 1:27 ` Chao Yu 0 siblings, 0 replies; 10+ messages in thread From: Chao Yu @ 2018-04-04 1:27 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2018/4/4 0:08, Jaegeuk Kim wrote: > On 04/03, Chao Yu wrote: >> On 2018/4/3 13:23, Jaegeuk Kim wrote: >>> On 04/03, Chao Yu wrote: >>>> On 2018/3/31 0:30, Jaegeuk Kim wrote: >>>>> Change log from v1: >>>>> - add more description >>>>> >>>>> This fixes xfstests/generic/392. >>>>> >>>>> The failure was caused by different times between 1) one marked in the last >>>>> fsync(2) call and 2) the other given by roll-forward recovery after power-cut. >>>>> The reason was that we skipped updating inode block at 1), since its i_size >>>>> was recoverable along with 4KB-aligned data writes, which was fixed by: >>>>> "f2fs: fix a wrong condition in f2fs_skip_inode_update" >>>>> >>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>>>> --- >>>>> fs/f2fs/f2fs.h | 15 +++++++++++++++ >>>>> fs/f2fs/inode.c | 4 ++++ >>>>> 2 files changed, 19 insertions(+) >>>>> >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>> index 000f93f6767e..675c39d85111 100644 >>>>> --- a/fs/f2fs/f2fs.h >>>>> +++ b/fs/f2fs/f2fs.h >>>>> @@ -664,6 +664,7 @@ struct f2fs_inode_info { >>>>> kprojid_t i_projid; /* id for project quota */ >>>>> int i_inline_xattr_size; /* inline xattr size */ >>>>> struct timespec i_crtime; /* inode creation time */ >>>>> + struct timespec i_disk_time[4]; /* inode disk times */ >>>>> }; >>>>> >>>>> static inline void get_extent_info(struct extent_info *ext, >>>>> @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) >>>>> f2fs_mark_inode_dirty_sync(inode, true); >>>>> } >>>>> >>>>> +static inline bool time_equal(struct timespec a, struct timespec b) >>>>> +{ >>>>> + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); >>>>> +} >>>> >>>> We can use existing timespec_equal in <linux/time.h> instead of defining a >>>> duplicated one? >>> >>> It is defined with const parameters. >> >> I didn't get it, const keyword can used to protect parameter not to be changed >> in that function, that will be more safe, so it will be better to use that one? > > Oh, I just made a mistake when testing timespec_equal(). > I really need more recess time. :P Work is hard, right, we'd better keep having enough rest. :) > > Change log from v3: > - use timespec_equal() > > This fixes xfstests/generic/392. > > The failure was caused by different times between 1) one marked in the last > fsync(2) call and 2) the other given by roll-forward recovery after power-cut. > The reason was that we skipped updating inode block at 1), since its i_size > was recoverable along with 4KB-aligned data writes, which was fixed by: > "f2fs: fix a wrong condition in f2fs_skip_inode_update" > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > --- > fs/f2fs/f2fs.h | 11 +++++++++++ > fs/f2fs/inode.c | 8 ++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 7102d3965fea..1df7f10476d6 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -664,6 +664,7 @@ struct f2fs_inode_info { > kprojid_t i_projid; /* id for project quota */ > int i_inline_xattr_size; /* inline xattr size */ > struct timespec i_crtime; /* inode creation time */ > + struct timespec i_disk_time[4]; /* inode disk times */ > }; > > static inline void get_extent_info(struct extent_info *ext, > @@ -2474,6 +2475,16 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) > i_size_read(inode) & ~PAGE_MASK) > return false; > > + if (!timespec_equal(F2FS_I(inode)->i_disk_time, &inode->i_atime)) > + return false; > + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 1, &inode->i_ctime)) > + return false; > + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 2, &inode->i_mtime)) > + return false; > + if (!timespec_equal(F2FS_I(inode)->i_disk_time + 3, > + &F2FS_I(inode)->i_crtime)) > + return false; > + > down_read(&F2FS_I(inode)->i_sem); > ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); > up_read(&F2FS_I(inode)->i_sem); > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index 401f09ccce7e..e0d9e8f27ed2 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -284,6 +284,10 @@ static int do_read_inode(struct inode *inode) > fi->i_crtime.tv_nsec = le32_to_cpu(ri->i_crtime_nsec); > } > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; > f2fs_put_page(node_page, 1); > > stat_inc_inline_xattr(inode); > @@ -444,6 +448,10 @@ void update_inode(struct inode *inode, struct page *node_page) > if (inode->i_nlink == 0) > clear_inline_node(node_page); > > + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; > + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; > + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; > + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; > } > > void update_inode_page(struct inode *inode) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: remain written times to update inode during fsync 2018-03-30 16:30 ` [PATCH v2] " Jaegeuk Kim 2018-04-03 2:29 ` Chao Yu @ 2018-04-03 5:22 ` Jaegeuk Kim 1 sibling, 0 replies; 10+ messages in thread From: Jaegeuk Kim @ 2018-04-03 5:22 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel Change log from v2: - update do_read_inode Change log from v1: - add more description This fixes xfstests/generic/392. The failure was caused by different times between 1) one marked in the last fsync(2) call and 2) the other given by roll-forward recovery after power-cut. The reason was that we skipped updating inode block at 1), since its i_size was recoverable along with 4KB-aligned data writes, which was fixed by: "f2fs: fix a wrong condition in f2fs_skip_inode_update" Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/f2fs.h | 15 +++++++++++++++ fs/f2fs/inode.c | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 7102d3965fea..33be61b19a5f 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -664,6 +664,7 @@ struct f2fs_inode_info { kprojid_t i_projid; /* id for project quota */ int i_inline_xattr_size; /* inline xattr size */ struct timespec i_crtime; /* inode creation time */ + struct timespec i_disk_time[4]; /* inode disk times */ }; static inline void get_extent_info(struct extent_info *ext, @@ -2457,6 +2458,11 @@ static inline void clear_file(struct inode *inode, int type) f2fs_mark_inode_dirty_sync(inode, true); } +static inline bool time_equal(struct timespec a, struct timespec b) +{ + return (a.tv_sec == b.tv_sec) && (a.tv_nsec == b.tv_nsec); +} + static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) { bool ret; @@ -2474,6 +2480,15 @@ static inline bool f2fs_skip_inode_update(struct inode *inode, int dsync) i_size_read(inode) & ~PAGE_MASK) return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[0], inode->i_atime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[1], inode->i_ctime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[2], inode->i_mtime)) + return false; + if (!time_equal(F2FS_I(inode)->i_disk_time[3], F2FS_I(inode)->i_crtime)) + return false; + down_read(&F2FS_I(inode)->i_sem); ret = F2FS_I(inode)->last_disk_size == i_size_read(inode); up_read(&F2FS_I(inode)->i_sem); diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 401f09ccce7e..e0d9e8f27ed2 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -284,6 +284,10 @@ static int do_read_inode(struct inode *inode) fi->i_crtime.tv_nsec = le32_to_cpu(ri->i_crtime_nsec); } + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; f2fs_put_page(node_page, 1); stat_inc_inline_xattr(inode); @@ -444,6 +448,10 @@ void update_inode(struct inode *inode, struct page *node_page) if (inode->i_nlink == 0) clear_inline_node(node_page); + F2FS_I(inode)->i_disk_time[0] = inode->i_atime; + F2FS_I(inode)->i_disk_time[1] = inode->i_ctime; + F2FS_I(inode)->i_disk_time[2] = inode->i_mtime; + F2FS_I(inode)->i_disk_time[3] = F2FS_I(inode)->i_crtime; } void update_inode_page(struct inode *inode) -- 2.15.0.531.g2ccb3012c9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-04 1:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-30 5:51 [PATCH] f2fs: remain written times to update inode during fsync Jaegeuk Kim 2018-03-30 10:54 ` Chao Yu 2018-03-30 16:31 ` Jaegeuk Kim 2018-03-30 16:30 ` [PATCH v2] " Jaegeuk Kim 2018-04-03 2:29 ` Chao Yu 2018-04-03 5:23 ` Jaegeuk Kim 2018-04-03 7:17 ` Chao Yu 2018-04-03 16:08 ` Jaegeuk Kim 2018-04-04 1:27 ` Chao Yu 2018-04-03 5:22 ` [f2fs-dev] [PATCH v3] " Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).