Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On 6/21/23 23:45, Jeff Layton wrote: > struct timespec64 has unused bits in the tv_nsec field that can be used > for other purposes. In future patches, we're going to change how the > inode->i_ctime is accessed in certain inodes in order to make use of > them. In order to do that safely though, we'll need to eradicate raw > accesses of the inode->i_ctime field from the kernel. > > Add new accessor functions for the ctime that we can use to replace them. > > Signed-off-by: Jeff Layton [...] > +/** > + * inode_ctime_peek - fetch the current ctime from the inode > + * @inode: inode from which to fetch ctime > + * > + * Grab the current ctime from the inode and return it. > + */ > +static inline struct timespec64 inode_ctime_peek(const struct inode *inode) To be consistent with inode_ctime_set(), why not call this one inode_ctime_get() ? Also, inode_set_ctime() & inode_get_ctime() may be a little more natural. But no strong opinion about that though. -- Damien Le Moal Western Digital Research ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On 6/21/2023 10:45 AM, Jeff Layton wrote:
struct timespec64 has unused bits in the tv_nsec field that can be used
for other purposes. In future patches, we're going to change how the
inode->i_ctime is accessed in certain inodes in order to make use of
them. In order to do that safely though, we'll need to eradicate raw
accesses of the inode->i_ctime field from the kernel.
Add new accessor functions for the ctime that we can use to replace them.
Signed-off-by: Jeff Layton
---
fs/inode.c | 16 ++
include/linux/fs.h | 53 +-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index d37fad91c8da..c005e7328fbb 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
}
EXPORT_SYMBOL(current_time);
+/**
+ * inode_ctime_set_current - set the ctime to current_time
+ * @inode: inode
+ *
+ * Set the inode->i_ctime to the current value for the inode. Returns
+ * the current value that was assigned to i_ctime.
+ */
+struct timespec64 inode_ctime_set_current(struct inode *inode)
+{
+ struct timespec64 now = current_time(inode);
+
+ inode_set_ctime(inode, now);
+ return now;
+}
+EXPORT_SYMBOL(inode_ctime_set_current);
+
/**
* in_group_or_capable - check whether caller is CAP_FSETID privileged
* @idmap:idmap of the mount @inode was found from
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6867512907d6..9afb30606373 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
super_block *sb,
kgid_has_mapping(fs_userns, kgid);
}
-extern struct timespec64 current_time(struct inode *inode);
+struct timespec64 current_time(struct inode *inode);
+struct timespec64 inode_ctime_set_current(struct inode *inode);
+
+/**
+ * inode_ctime_peek - fetch the current ctime from the inode
+ * @inode: inode from which to fetch ctime
+ *
+ * Grab the current ctime from the inode and return it.
+ */
+static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
+{
+ return inode->i_ctime;
+}
+
+/**
+ * inode_ctime_set - set the ctime in the inode to the given value
+ * @inode: inode in which to set the ctime
+ * @ts: timespec value to set the ctime
+ *
+ * Set the ctime in @inode to @ts.
+ */
+static inline struct timespec64 inode_ctime_set(struct inode *inode, struct
timespec64 ts)
+{
+ inode->i_ctime = ts;
+ return ts;
+}
+
+/**
+ * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
I'm curious about why you choose to split the tv_sec and tv_nsec
set_ functions. Do any callers not set them both? Wouldn't a
single call enable a more atomic behavior someday?
inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
(or simply initialize a timespec64 and use inode_ctime_spec() )
Tom.
+ * @inode: inode in which to set the ctime
+ * @sec: value to set the tv_sec field
+ *
+ * Set the sec field in the ctime. Returns @sec.
+ */
+static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
+{
+ inode->i_ctime.tv_sec = sec;
+ return sec;
+}
+
+/**
+ * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @nsec: value to set the tv_nsec field
+ *
+ * Set the nsec field in the ctime. Returns @nsec.
+ */
+static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
+{
+ inode->i_ctime.tv_nsec = nsec;
+ return nsec;
+}
/*
* Snapshotting support.
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 00/79] fs: new accessors for inode->i_ctime
On Wed, 2023-06-21 at 15:21 -0400, Steven Rostedt wrote: > On Wed, 21 Jun 2023 10:45:05 -0400 > Jeff Layton wrote: > > > Most of this conversion was done via coccinelle, with a few of the more > > non-standard accesses done by hand. There should be no behavioral > > changes with this set. That will come later, as we convert individual > > filesystems to use multigrain timestamps. > > BTW, Linus has suggested to me that whenever a conccinelle script is used, > it should be included in the change log. > Ok, here's what I have. I note again that my usage of coccinelle is pretty primitive, so I ended up doing a fair bit of by-hand fixing after applying these. Given the way that this change is broken up into 77 patches by subsystem, to which changelogs should I add it? I could add it to the "infrastructure" patch, but that's the one where I _didn't_ use it. Maybe to patch #79 (the one that renames i_ctime)? 8<-- @@ expression inode; @@ - inode->i_ctime = current_time(inode) + inode_set_current_ctime(inode) @@ expression inode; @@ - inode->i_ctime = inode->i_mtime = current_time(inode) + inode->i_mtime = inode_set_current_ctime(inode) @@ struct inode *inode; expression value; @@ - inode->i_ctime = value; + inode_set_ctime(inode, value); @@ struct inode *inode; expression val; @@ - inode->i_ctime.tv_sec = val + inode_set_ctime_sec(inode, val) @@ struct inode *inode; expression val; @@ - inode->i_ctime.tv_nsec = val + inode_set_ctime_nsec(inode, val) @@ struct inode *inode; @@ - inode->i_ctime + inode_ctime_peek(inode) ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 00/79] fs: new accessors for inode->i_ctime
On Wed, 21 Jun 2023 10:45:05 -0400 Jeff Layton wrote: > Most of this conversion was done via coccinelle, with a few of the more > non-standard accesses done by hand. There should be no behavioral > changes with this set. That will come later, as we convert individual > filesystems to use multigrain timestamps. BTW, Linus has suggested to me that whenever a conccinelle script is used, it should be included in the change log. -- Steve ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On 6/21/2023 2:01 PM, Jeff Layton wrote:
On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
On 6/21/2023 10:45 AM, Jeff Layton wrote:
struct timespec64 has unused bits in the tv_nsec field that can be used
for other purposes. In future patches, we're going to change how the
inode->i_ctime is accessed in certain inodes in order to make use of
them. In order to do that safely though, we'll need to eradicate raw
accesses of the inode->i_ctime field from the kernel.
Add new accessor functions for the ctime that we can use to replace them.
Signed-off-by: Jeff Layton
---
fs/inode.c | 16 ++
include/linux/fs.h | 53 +-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index d37fad91c8da..c005e7328fbb 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
}
EXPORT_SYMBOL(current_time);
+/**
+ * inode_ctime_set_current - set the ctime to current_time
+ * @inode: inode
+ *
+ * Set the inode->i_ctime to the current value for the inode. Returns
+ * the current value that was assigned to i_ctime.
+ */
+struct timespec64 inode_ctime_set_current(struct inode *inode)
+{
+ struct timespec64 now = current_time(inode);
+
+ inode_set_ctime(inode, now);
+ return now;
+}
+EXPORT_SYMBOL(inode_ctime_set_current);
+
/**
* in_group_or_capable - check whether caller is CAP_FSETID privileged
* @idmap: idmap of the mount @inode was found from
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6867512907d6..9afb30606373 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
super_block *sb,
kgid_has_mapping(fs_userns, kgid);
}
-extern struct timespec64 current_time(struct inode *inode);
+struct timespec64 current_time(struct inode *inode);
+struct timespec64 inode_ctime_set_current(struct inode *inode);
+
+/**
+ * inode_ctime_peek - fetch the current ctime from the inode
+ * @inode: inode from which to fetch ctime
+ *
+ * Grab the current ctime from the inode and return it.
+ */
+static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
+{
+ return inode->i_ctime;
+}
+
+/**
+ * inode_ctime_set - set the ctime in the inode to the given value
+ * @inode: inode in which to set the ctime
+ * @ts: timespec value to set the ctime
+ *
+ * Set the ctime in @inode to @ts.
+ */
+static inline struct timespec64 inode_ctime_set(struct inode *inode, struct
timespec64 ts)
+{
+ inode->i_ctime = ts;
+ return ts;
+}
+
+/**
+ * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
I'm curious about why you choose to split the tv_sec and tv_nsec
set_ functions. Do any callers not set them both? Wouldn't a
single call enable a more atomic behavior someday?
inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
(or simply initialize a timespec64 and use inode_ctime_spec() )
Yes, quite a few places set the fields individually. For example, when
loading a value from disk that doesn't have sufficient granularity to
set the nsecs field to anything but 0.
Well, they still need to set the tv_nsec so they could just pass 0.
But ok.
Could I have done it by declaring a local timespec64 variable and just
use the inode_ctime_set function in these places? Absolutely.
That's a bit more difficult to handle with coccinelle though. If someone
wants to suggest a way to do that without having to change all of these
call sites manually, then I'm open to redoing the set.
That might be better left for a later cleanup though.
Acked-by: Tom Talpey
+ * @inode: inode in which to set the ctime
+ * @sec: value to set the tv_sec field
+ *
+ * Set the sec field in the ctime. Returns @sec.
+ */
+static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
+{
+ inode->i_ctime.tv_sec = sec;
+ return sec;
+}
+
+/**
+ * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @nsec: value to set the tv_nsec field
+ *
+ * Set the nsec field in the ctime. Returns @nsec.
+ */
+static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
+{
+ inode->i_ctime.tv_nsec = nsec;
+ return nsec;
+}
/*
* Snapshotting support.
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On Wed, 2023-06-21 at 14:19 -0400, Tom Talpey wrote:
> On 6/21/2023 2:01 PM, Jeff Layton wrote:
> > On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
> > > On 6/21/2023 10:45 AM, Jeff Layton wrote:
> > > > struct timespec64 has unused bits in the tv_nsec field that can be used
> > > > for other purposes. In future patches, we're going to change how the
> > > > inode->i_ctime is accessed in certain inodes in order to make use of
> > > > them. In order to do that safely though, we'll need to eradicate raw
> > > > accesses of the inode->i_ctime field from the kernel.
> > > >
> > > > Add new accessor functions for the ctime that we can use to replace
> > > > them.
> > > >
> > > > Signed-off-by: Jeff Layton
> > > > ---
> > > >fs/inode.c | 16 ++
> > > >include/linux/fs.h | 53
> > > > +-
> > > >2 files changed, 68 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/inode.c b/fs/inode.c
> > > > index d37fad91c8da..c005e7328fbb 100644
> > > > --- a/fs/inode.c
> > > > +++ b/fs/inode.c
> > > > @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode
> > > > *inode)
> > > >}
> > > >EXPORT_SYMBOL(current_time);
> > > >
> > > > +/**
> > > > + * inode_ctime_set_current - set the ctime to current_time
> > > > + * @inode: inode
> > > > + *
> > > > + * Set the inode->i_ctime to the current value for the inode. Returns
> > > > + * the current value that was assigned to i_ctime.
> > > > + */
> > > > +struct timespec64 inode_ctime_set_current(struct inode *inode)
> > > > +{
> > > > + struct timespec64 now = current_time(inode);
> > > > +
> > > > + inode_set_ctime(inode, now);
> > > > + return now;
> > > > +}
> > > > +EXPORT_SYMBOL(inode_ctime_set_current);
> > > > +
> > > >/**
> > > > * in_group_or_capable - check whether caller is CAP_FSETID
> > > > privileged
> > > > * @idmap: idmap of the mount @inode was found from
> > > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > > index 6867512907d6..9afb30606373 100644
> > > > --- a/include/linux/fs.h
> > > > +++ b/include/linux/fs.h
> > > > @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
> > > > super_block *sb,
> > > >kgid_has_mapping(fs_userns, kgid);
> > > >}
> > > >
> > > > -extern struct timespec64 current_time(struct inode *inode);
> > > > +struct timespec64 current_time(struct inode *inode);
> > > > +struct timespec64 inode_ctime_set_current(struct inode *inode);
> > > > +
> > > > +/**
> > > > + * inode_ctime_peek - fetch the current ctime from the inode
> > > > + * @inode: inode from which to fetch ctime
> > > > + *
> > > > + * Grab the current ctime from the inode and return it.
> > > > + */
> > > > +static inline struct timespec64 inode_ctime_peek(const struct inode
> > > > *inode)
> > > > +{
> > > > + return inode->i_ctime;
> > > > +}
> > > > +
> > > > +/**
> > > > + * inode_ctime_set - set the ctime in the inode to the given value
> > > > + * @inode: inode in which to set the ctime
> > > > + * @ts: timespec value to set the ctime
> > > > + *
> > > > + * Set the ctime in @inode to @ts.
> > > > + */
> > > > +static inline struct timespec64 inode_ctime_set(struct inode *inode,
> > > > struct timespec64 ts)
> > > > +{
> > > > + inode->i_ctime = ts;
> > > > + return ts;
> > > > +}
> > > > +
> > > > +/**
> > > > + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
> > >
> > > I'm curious about why you choose to split the tv_sec and tv_nsec
> > > set_ functions. Do any callers not set them both? Wouldn't a
> > > single call enable a more atomic behavior someday?
> > >
> > > inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
> > >
> > > (or simply initialize a timespec64 and use inode_ctime_spec() )
> > >
> >
> > Yes, quite a few places set the fields individually. For example, when
> > loading a value from disk that doesn't have sufficient granularity to
> > set the nsecs field to anything but 0.
>
> Well, they still need to set the tv_nsec so they could just pass 0.
> But ok.
>
Sure. The difficulty is in trying to do this in an automated way. For
instance, look at the hfsplus patch; it has separate assignments in
place already:
- result->i_ctime.tv_sec = result->i_mtime.tv_sec =
result->i_atime.tv_sec = local_to_gmt(dir->i_sb,
le32_to_cpu(dee.creation_date));
- result->i_ctime.tv_nsec = 0;
+ inode_ctime_set_sec(result,
+ result->i_mtime.tv_sec = result->i_atime.tv_sec =
local_to_gmt(dir->i_sb, le32_to_cpu(dee.creation_date)));
+ inode_ctime_set_nsec(result, 0);
Granted the new code is pretty ugly, but it compiles!
Transforming that into what you're suggesting is a tougher proposition
to do with coccinelle. I didn't see a way to conditionally catch cases
like this, declare a new variable in the appropriate spot and then
transform two assignments (that may
Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
> On 6/21/2023 10:45 AM, Jeff Layton wrote:
> > struct timespec64 has unused bits in the tv_nsec field that can be used
> > for other purposes. In future patches, we're going to change how the
> > inode->i_ctime is accessed in certain inodes in order to make use of
> > them. In order to do that safely though, we'll need to eradicate raw
> > accesses of the inode->i_ctime field from the kernel.
> >
> > Add new accessor functions for the ctime that we can use to replace them.
> >
> > Signed-off-by: Jeff Layton
> > ---
> > fs/inode.c | 16 ++
> > include/linux/fs.h | 53 +-
> > 2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/inode.c b/fs/inode.c
> > index d37fad91c8da..c005e7328fbb 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> > }
> > EXPORT_SYMBOL(current_time);
> >
> > +/**
> > + * inode_ctime_set_current - set the ctime to current_time
> > + * @inode: inode
> > + *
> > + * Set the inode->i_ctime to the current value for the inode. Returns
> > + * the current value that was assigned to i_ctime.
> > + */
> > +struct timespec64 inode_ctime_set_current(struct inode *inode)
> > +{
> > + struct timespec64 now = current_time(inode);
> > +
> > + inode_set_ctime(inode, now);
> > + return now;
> > +}
> > +EXPORT_SYMBOL(inode_ctime_set_current);
> > +
> > /**
> >* in_group_or_capable - check whether caller is CAP_FSETID privileged
> >* @idmap:idmap of the mount @inode was found from
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 6867512907d6..9afb30606373 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
> > super_block *sb,
> >kgid_has_mapping(fs_userns, kgid);
> > }
> >
> > -extern struct timespec64 current_time(struct inode *inode);
> > +struct timespec64 current_time(struct inode *inode);
> > +struct timespec64 inode_ctime_set_current(struct inode *inode);
> > +
> > +/**
> > + * inode_ctime_peek - fetch the current ctime from the inode
> > + * @inode: inode from which to fetch ctime
> > + *
> > + * Grab the current ctime from the inode and return it.
> > + */
> > +static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
> > +{
> > + return inode->i_ctime;
> > +}
> > +
> > +/**
> > + * inode_ctime_set - set the ctime in the inode to the given value
> > + * @inode: inode in which to set the ctime
> > + * @ts: timespec value to set the ctime
> > + *
> > + * Set the ctime in @inode to @ts.
> > + */
> > +static inline struct timespec64 inode_ctime_set(struct inode *inode,
> > struct timespec64 ts)
> > +{
> > + inode->i_ctime = ts;
> > + return ts;
> > +}
> > +
> > +/**
> > + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
>
> I'm curious about why you choose to split the tv_sec and tv_nsec
> set_ functions. Do any callers not set them both? Wouldn't a
> single call enable a more atomic behavior someday?
>
>inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
>
> (or simply initialize a timespec64 and use inode_ctime_spec() )
>
Yes, quite a few places set the fields individually. For example, when
loading a value from disk that doesn't have sufficient granularity to
set the nsecs field to anything but 0.
Could I have done it by declaring a local timespec64 variable and just
use the inode_ctime_set function in these places? Absolutely.
That's a bit more difficult to handle with coccinelle though. If someone
wants to suggest a way to do that without having to change all of these
call sites manually, then I'm open to redoing the set.
That might be better left for a later cleanup though.
> > + * @inode: inode in which to set the ctime
> > + * @sec: value to set the tv_sec field
> > + *
> > + * Set the sec field in the ctime. Returns @sec.
> > + */
> > +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t
> > sec)
> > +{
> > + inode->i_ctime.tv_sec = sec;
> > + return sec;
> > +}
> > +
> > +/**
> > + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> > + * @inode: inode in which to set the ctime
> > + * @nsec: value to set the tv_nsec field
> > + *
> > + * Set the nsec field in the ctime. Returns @nsec.
> > + */
> > +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> > +{
> > + inode->i_ctime.tv_nsec = nsec;
> > + return nsec;
> > +}
> >
> > /*
> >* Snapshotting support.
--
Jeff Layton
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
On Wed 21-06-23 10:45:06, Jeff Layton wrote:
> struct timespec64 has unused bits in the tv_nsec field that can be used
> for other purposes. In future patches, we're going to change how the
> inode->i_ctime is accessed in certain inodes in order to make use of
> them. In order to do that safely though, we'll need to eradicate raw
> accesses of the inode->i_ctime field from the kernel.
>
> Add new accessor functions for the ctime that we can use to replace them.
>
> Signed-off-by: Jeff Layton
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara
Honza
> ---
> fs/inode.c | 16 ++
> include/linux/fs.h | 53 +-
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index d37fad91c8da..c005e7328fbb 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> }
> EXPORT_SYMBOL(current_time);
>
> +/**
> + * inode_ctime_set_current - set the ctime to current_time
> + * @inode: inode
> + *
> + * Set the inode->i_ctime to the current value for the inode. Returns
> + * the current value that was assigned to i_ctime.
> + */
> +struct timespec64 inode_ctime_set_current(struct inode *inode)
> +{
> + struct timespec64 now = current_time(inode);
> +
> + inode_set_ctime(inode, now);
> + return now;
> +}
> +EXPORT_SYMBOL(inode_ctime_set_current);
> +
> /**
> * in_group_or_capable - check whether caller is CAP_FSETID privileged
> * @idmap: idmap of the mount @inode was found from
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6867512907d6..9afb30606373 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
> super_block *sb,
> kgid_has_mapping(fs_userns, kgid);
> }
>
> -extern struct timespec64 current_time(struct inode *inode);
> +struct timespec64 current_time(struct inode *inode);
> +struct timespec64 inode_ctime_set_current(struct inode *inode);
> +
> +/**
> + * inode_ctime_peek - fetch the current ctime from the inode
> + * @inode: inode from which to fetch ctime
> + *
> + * Grab the current ctime from the inode and return it.
> + */
> +static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
> +{
> + return inode->i_ctime;
> +}
> +
> +/**
> + * inode_ctime_set - set the ctime in the inode to the given value
> + * @inode: inode in which to set the ctime
> + * @ts: timespec value to set the ctime
> + *
> + * Set the ctime in @inode to @ts.
> + */
> +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct
> timespec64 ts)
> +{
> + inode->i_ctime = ts;
> + return ts;
> +}
> +
> +/**
> + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
> + * @inode: inode in which to set the ctime
> + * @sec: value to set the tv_sec field
> + *
> + * Set the sec field in the ctime. Returns @sec.
> + */
> +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
> +{
> + inode->i_ctime.tv_sec = sec;
> + return sec;
> +}
> +
> +/**
> + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> + * @inode: inode in which to set the ctime
> + * @nsec: value to set the tv_nsec field
> + *
> + * Set the nsec field in the ctime. Returns @nsec.
> + */
> +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> +{
> + inode->i_ctime.tv_nsec = nsec;
> + return nsec;
> +}
>
> /*
> * Snapshotting support.
> --
> 2.41.0
>
--
Jan Kara
SUSE Labs, CR
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] Trying to use compression options in f2fs
Greetings, I'm trying to compare the performance of the various compression options in f2fs versus the default settings, on an SD card. Here's what I'm doing: * Creating the filesystem: mkfs.f2fs -f -l data -O compression,extra_attr /dev/mmcblk0p3 * Mounting the filesystem: mount -o compress_algorithm=zstd:22 /dev/mmcblk0p3 /mnt/data I'm running the tests on the Beaglebone Black board with a single-core 1GHz ARM CPU, using Linux 6.3.6. I'm using mkfs.f2fs 1.15.0 (2022-05-13), built by Buildroot. However, I see no significant difference in terms of disk usage, read and write time, typically when I write and read an ARM root filesystem (from the Raspberry Pi Lite distro), compared to creating and mounting the filesystem with no special options. On Btrfs, the differences between compression options are really important. I chose zstd:22 because I expected it to be the slowest choice in terms of compression. I'm expecting all files to be compressed (if they are compressible) by default. Am I missing anything? Thanks in advance, Michael. -- Michael Opdenacker, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 79/79] fs: rename i_ctime field to __i_ctime
Now that everything in-tree is converted to use the accessor functions,
rename the i_ctime field in the inode to make its accesses more
self-documenting.
Signed-off-by: Jeff Layton
---
include/linux/fs.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9afb30606373..2ca46c532b49 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -642,7 +642,7 @@ struct inode {
loff_t i_size;
struct timespec64 i_atime;
struct timespec64 i_mtime;
- struct timespec64 i_ctime;
+ struct timespec64 __i_ctime; /* use inode_ctime_* accessors! */
spinlock_t i_lock; /* i_blocks, i_bytes, maybe i_size */
unsigned short i_bytes;
u8 i_blkbits;
@@ -1485,7 +1485,7 @@ struct timespec64 inode_ctime_set_current(struct inode
*inode);
*/
static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
{
- return inode->i_ctime;
+ return inode->__i_ctime;
}
/**
@@ -1497,7 +1497,7 @@ static inline struct timespec64 inode_ctime_peek(const
struct inode *inode)
*/
static inline struct timespec64 inode_ctime_set(struct inode *inode, struct
timespec64 ts)
{
- inode->i_ctime = ts;
+ inode->__i_ctime = ts;
return ts;
}
@@ -1510,7 +1510,7 @@ static inline struct timespec64 inode_ctime_set(struct
inode *inode, struct time
*/
static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
{
- inode->i_ctime.tv_sec = sec;
+ inode->__i_ctime.tv_sec = sec;
return sec;
}
@@ -1523,7 +1523,7 @@ static inline time64_t inode_ctime_set_sec(struct inode
*inode, time64_t sec)
*/
static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
{
- inode->i_ctime.tv_nsec = nsec;
+ inode->__i_ctime.tv_nsec = nsec;
return nsec;
}
--
2.41.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 30/79] f2fs: switch to new ctime accessors
In later patches, we're going to change how the ctime.tv_nsec field is
utilized. Switch to using accessor functions instead of raw accesses of
inode->i_ctime.
Signed-off-by: Jeff Layton
---
fs/f2fs/dir.c | 8
fs/f2fs/f2fs.h | 5 -
fs/f2fs/file.c | 16
fs/f2fs/inline.c | 2 +-
fs/f2fs/inode.c| 10 +-
fs/f2fs/namei.c| 12 ++--
fs/f2fs/recovery.c | 4 ++--
fs/f2fs/super.c| 2 +-
fs/f2fs/xattr.c| 2 +-
9 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 887e55988450..54fa7285e2d6 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -455,7 +455,7 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry
*de,
de->file_type = fs_umode_to_ftype(inode->i_mode);
set_page_dirty(page);
- dir->i_mtime = dir->i_ctime = current_time(dir);
+ dir->i_mtime = inode_ctime_set_current(dir);
f2fs_mark_inode_dirty_sync(dir, false);
f2fs_put_page(page, 1);
}
@@ -609,7 +609,7 @@ void f2fs_update_parent_metadata(struct inode *dir, struct
inode *inode,
f2fs_i_links_write(dir, true);
clear_inode_flag(inode, FI_NEW_INODE);
}
- dir->i_mtime = dir->i_ctime = current_time(dir);
+ dir->i_mtime = inode_ctime_set_current(dir);
f2fs_mark_inode_dirty_sync(dir, false);
if (F2FS_I(dir)->i_current_depth != current_depth)
@@ -851,7 +851,7 @@ void f2fs_drop_nlink(struct inode *dir, struct inode *inode)
if (S_ISDIR(inode->i_mode))
f2fs_i_links_write(dir, false);
- inode->i_ctime = current_time(inode);
+ inode_ctime_set_current(inode);
f2fs_i_links_write(inode, false);
if (S_ISDIR(inode->i_mode)) {
@@ -912,7 +912,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry,
struct page *page,
}
f2fs_put_page(page, 1);
- dir->i_ctime = dir->i_mtime = current_time(dir);
+ dir->i_mtime = inode_ctime_set_current(dir);
f2fs_mark_inode_dirty_sync(dir, false);
if (inode)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7b9af2d51656..b0a0de41f823 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3302,9 +3302,12 @@ static inline void clear_file(struct inode *inode, int
type)
static inline bool f2fs_is_time_consistent(struct inode *inode)
{
+ struct timespec64 ctime;
+
if (!timespec64_equal(F2FS_I(inode)->i_disk_time, >i_atime))
return false;
- if (!timespec64_equal(F2FS_I(inode)->i_disk_time + 1, >i_ctime))
+ ctime = inode_ctime_peek(inode);
+ if (!timespec64_equal(F2FS_I(inode)->i_disk_time + 1, ))
return false;
if (!timespec64_equal(F2FS_I(inode)->i_disk_time + 2, >i_mtime))
return false;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 38688b5192ef..76c38cd89865 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -809,7 +809,7 @@ int f2fs_truncate(struct inode *inode)
if (err)
return err;
- inode->i_mtime = inode->i_ctime = current_time(inode);
+ inode->i_mtime = inode_ctime_set_current(inode);
f2fs_mark_inode_dirty_sync(inode, false);
return 0;
}
@@ -920,7 +920,7 @@ static void __setattr_copy(struct mnt_idmap *idmap,
if (ia_valid & ATTR_MTIME)
inode->i_mtime = attr->ia_mtime;
if (ia_valid & ATTR_CTIME)
- inode->i_ctime = attr->ia_ctime;
+ inode_ctime_set(inode, attr->ia_ctime);
if (ia_valid & ATTR_MODE) {
umode_t mode = attr->ia_mode;
vfsgid_t vfsgid = i_gid_into_vfsgid(idmap, inode);
@@ -1023,7 +1023,7 @@ int f2fs_setattr(struct mnt_idmap *idmap, struct dentry
*dentry,
return err;
spin_lock(_I(inode)->i_size_lock);
- inode->i_mtime = inode->i_ctime = current_time(inode);
+ inode->i_mtime = inode_ctime_set_current(inode);
F2FS_I(inode)->last_disk_size = i_size_read(inode);
spin_unlock(_I(inode)->i_size_lock);
}
@@ -1850,7 +1850,7 @@ static long f2fs_fallocate(struct file *file, int mode,
}
if (!ret) {
- inode->i_mtime = inode->i_ctime = current_time(inode);
+ inode->i_mtime = inode_ctime_set_current(inode);
f2fs_mark_inode_dirty_sync(inode, false);
f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
}
@@ -1952,7 +1952,7 @@ static int f2fs_setflags_common(struct inode *inode, u32
iflags, u32 mask)
else
clear_inode_flag(inode, FI_PROJ_INHERIT);
- inode->i_ctime = current_time(inode);
+ inode_ctime_set_current(inode);
f2fs_set_inode_flags(inode);
f2fs_mark_inode_dirty_sync(inode, true);
return 0;
@@ -3077,7 +3077,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32
projid)
[f2fs-dev] [PATCH 00/79] fs: new accessors for inode->i_ctime
I've been working on a patchset to change how the inode->i_ctime is accessed in order to give us conditional, high-res timestamps for the ctime and mtime. struct timespec64 has unused bits in it that we can use to implement this. In order to do that however, we need to wrap all accesses of inode->i_ctime to ensure that bits used as flags are appropriately handled. This patchset first adds some new inode_ctime_* accessor functions. It then converts all in-tree accesses of inode->i_ctime to use those new functions and then renames the i_ctime field to __i_ctime to help ensure that use of the accessors. Most of this conversion was done via coccinelle, with a few of the more non-standard accesses done by hand. There should be no behavioral changes with this set. That will come later, as we convert individual filesystems to use multigrain timestamps. Some of these patches depend on the set I sent recently to add missing ctime updates in various subsystems: https://lore.kernel.org/linux-fsdevel/20230612104524.17058-1-jlay...@kernel.org/T/#m25399f903cc9526e46b2e0f5a35713c80b52fde9 Since this patchset is so large, I'm only going to send individual conversion patches to the appropriate maintainers. Please send Acked-by's or Reviewed-by's if you can. The intent is to merge these as a set (probably in v6.6). Let me know if that causes conflicts though, and we can work it out. This is based on top of linux-next as of yesterday. Jeff Layton (79): fs: add ctime accessors infrastructure spufs: switch to new ctime accessors s390: switch to new ctime accessors binderfs: switch to new ctime accessors qib_fs: switch to new ctime accessors ibm: switch to new ctime accessors usb: switch to new ctime accessors 9p: switch to new ctime accessors adfs: switch to new ctime accessors affs: switch to new ctime accessors afs: switch to new ctime accessors fs: switch to new ctime accessors autofs: switch to new ctime accessors befs: switch to new ctime accessors bfs: switch to new ctime accessors btrfs: switch to new ctime accessors ceph: switch to new ctime accessors coda: switch to new ctime accessors configfs: switch to new ctime accessors cramfs: switch to new ctime accessors debugfs: switch to new ctime accessors devpts: switch to new ctime accessors ecryptfs: switch to new ctime accessors efivarfs: switch to new ctime accessors efs: switch to new ctime accessors erofs: switch to new ctime accessors exfat: switch to new ctime accessors ext2: switch to new ctime accessors ext4: switch to new ctime accessors f2fs: switch to new ctime accessors fat: switch to new ctime accessors freevxfs: switch to new ctime accessors fuse: switch to new ctime accessors gfs2: switch to new ctime accessors hfs: switch to new ctime accessors hfsplus: switch to new ctime accessors hostfs: switch to new ctime accessors hpfs: switch to new ctime accessors hugetlbfs: switch to new ctime accessors isofs: switch to new ctime accessors jffs2: switch to new ctime accessors jfs: switch to new ctime accessors kernfs: switch to new ctime accessors minix: switch to new ctime accessors nfs: switch to new ctime accessors nfsd: switch to new ctime accessors nilfs2: switch to new ctime accessors ntfs: switch to new ctime accessors ntfs3: switch to new ctime accessors ocfs2: switch to new ctime accessors omfs: switch to new ctime accessors openpromfs: switch to new ctime accessors orangefs: switch to new ctime accessors overlayfs: switch to new ctime accessors proc: switch to new ctime accessors pstore: switch to new ctime accessors qnx4: switch to new ctime accessors qnx6: switch to new ctime accessors ramfs: switch to new ctime accessors reiserfs: switch to new ctime accessors romfs: switch to new ctime accessors smb: switch to new ctime accessors squashfs: switch to new ctime accessors sysv: switch to new ctime accessors tracefs: switch to new ctime accessors ubifs: switch to new ctime accessors udf: switch to new ctime accessors ufs: switch to new ctime accessors vboxsf: switch to new ctime accessors xfs: switch to new ctime accessors zonefs: switch to new ctime accessors mqueue: switch to new ctime accessors bpf: switch to new ctime accessors shmem: switch to new ctime accessors rpc_pipefs: switch to new ctime accessors apparmor: switch to new ctime accessors security: switch to new ctime accessors selinux: switch to new ctime accessors fs: rename i_ctime field to __i_ctime arch/powerpc/platforms/cell/spufs/inode.c | 2 +- arch/s390/hypfs/inode.c | 4 +- drivers/android/binderfs.c| 8 +-- drivers/infiniband/hw/qib/qib_fs.c| 4 +- drivers/misc/ibmasm/ibmasmfs.c| 2 +- drivers/misc/ibmvmc.c | 2 +- drivers/usb/core/devio.c | 16 +++--- drivers/usb/gadget/function/f_fs.c| 6 +--
[f2fs-dev] [PATCH 01/79] fs: add ctime accessors infrastructure
struct timespec64 has unused bits in the tv_nsec field that can be used
for other purposes. In future patches, we're going to change how the
inode->i_ctime is accessed in certain inodes in order to make use of
them. In order to do that safely though, we'll need to eradicate raw
accesses of the inode->i_ctime field from the kernel.
Add new accessor functions for the ctime that we can use to replace them.
Signed-off-by: Jeff Layton
---
fs/inode.c | 16 ++
include/linux/fs.h | 53 +-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index d37fad91c8da..c005e7328fbb 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
}
EXPORT_SYMBOL(current_time);
+/**
+ * inode_ctime_set_current - set the ctime to current_time
+ * @inode: inode
+ *
+ * Set the inode->i_ctime to the current value for the inode. Returns
+ * the current value that was assigned to i_ctime.
+ */
+struct timespec64 inode_ctime_set_current(struct inode *inode)
+{
+ struct timespec64 now = current_time(inode);
+
+ inode_set_ctime(inode, now);
+ return now;
+}
+EXPORT_SYMBOL(inode_ctime_set_current);
+
/**
* in_group_or_capable - check whether caller is CAP_FSETID privileged
* @idmap: idmap of the mount @inode was found from
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6867512907d6..9afb30606373 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct
super_block *sb,
kgid_has_mapping(fs_userns, kgid);
}
-extern struct timespec64 current_time(struct inode *inode);
+struct timespec64 current_time(struct inode *inode);
+struct timespec64 inode_ctime_set_current(struct inode *inode);
+
+/**
+ * inode_ctime_peek - fetch the current ctime from the inode
+ * @inode: inode from which to fetch ctime
+ *
+ * Grab the current ctime from the inode and return it.
+ */
+static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
+{
+ return inode->i_ctime;
+}
+
+/**
+ * inode_ctime_set - set the ctime in the inode to the given value
+ * @inode: inode in which to set the ctime
+ * @ts: timespec value to set the ctime
+ *
+ * Set the ctime in @inode to @ts.
+ */
+static inline struct timespec64 inode_ctime_set(struct inode *inode, struct
timespec64 ts)
+{
+ inode->i_ctime = ts;
+ return ts;
+}
+
+/**
+ * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @sec: value to set the tv_sec field
+ *
+ * Set the sec field in the ctime. Returns @sec.
+ */
+static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
+{
+ inode->i_ctime.tv_sec = sec;
+ return sec;
+}
+
+/**
+ * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @nsec: value to set the tv_nsec field
+ *
+ * Set the nsec field in the ctime. Returns @nsec.
+ */
+static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
+{
+ inode->i_ctime.tv_nsec = nsec;
+ return nsec;
+}
/*
* Snapshotting support.
--
2.41.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/2] f2fs: truncate pages if move file range success
On 2023/6/21 17:43, Yunlei He wrote: If move file range success, it should remove old data from src and dst page cache. Signed-off-by: Yunlei He Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/2 v2] f2fs: update mtime and ctime in move file range method
On 2023/6/21 17:43, Yunlei He wrote:
Mtime and ctime stay old value without update after move
file range ioctl. This patch add time update.
Signed-off-by: Yunlei He
---
v2:
-update both src and dst inode
fs/f2fs/file.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index cb42d8464ad9..4adcf62e2665 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2884,8 +2884,22 @@ static int f2fs_move_file_range(struct file *file_in,
loff_t pos_in,
if (src != dst)
f2fs_up_write(_I(dst)->i_gc_rwsem[WRITE]);
+
Unneeded blank line.
out_src:
f2fs_up_write(_I(src)->i_gc_rwsem[WRITE]);
+
+ if (!ret) {
+ src->i_mtime = src->i_ctime = current_time(src);
+ f2fs_mark_inode_dirty_sync(src, false);
+
+ if (src != dst) {
+ dst->i_mtime = dst->i_ctime = current_time(dst);
+ f2fs_mark_inode_dirty_sync(dst, false);
+ }
+
+ f2fs_update_time(F2FS_I_SB(src), REQ_TIME);
f2fs_update_time(sbi, REQ_TIME);
Thanks,
+ }
+
out_unlock:
if (src != dst)
inode_unlock(dst);
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/2 v2] f2fs: update mtime and ctime in move file range method
Mtime and ctime stay old value without update after move
file range ioctl. This patch add time update.
Signed-off-by: Yunlei He
---
v2:
-update both src and dst inode
fs/f2fs/file.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index cb42d8464ad9..4adcf62e2665 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2884,8 +2884,22 @@ static int f2fs_move_file_range(struct file *file_in,
loff_t pos_in,
if (src != dst)
f2fs_up_write(_I(dst)->i_gc_rwsem[WRITE]);
+
out_src:
f2fs_up_write(_I(src)->i_gc_rwsem[WRITE]);
+
+ if (!ret) {
+ src->i_mtime = src->i_ctime = current_time(src);
+ f2fs_mark_inode_dirty_sync(src, false);
+
+ if (src != dst) {
+ dst->i_mtime = dst->i_ctime = current_time(dst);
+ f2fs_mark_inode_dirty_sync(dst, false);
+ }
+
+ f2fs_update_time(F2FS_I_SB(src), REQ_TIME);
+ }
+
out_unlock:
if (src != dst)
inode_unlock(dst);
--
2.40.1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/2] f2fs: truncate pages if move file range success
If move file range success, it should remove old data from src and dst page cache. Signed-off-by: Yunlei He --- fs/f2fs/file.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 4adcf62e2665..25ef36f2376a 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2897,6 +2897,9 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in, f2fs_mark_inode_dirty_sync(dst, false); } + truncate_pagecache_range(src, pos_in, pos_in + len - 1); + truncate_pagecache_range(dst, pos_out, pos_out + len - 1); + f2fs_update_time(F2FS_I_SB(src), REQ_TIME); } -- 2.40.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v8] fsck.f2fs: Detect and fix looped node chain efficiently
Hi Chao,
Sorry for replying late, I have send patch "fsck.f2fs: refactor looped
node chain detetected logic for cleanup" as you suggested.
Thanks.
On 2023/6/17 10:20, Chao Yu wrote:
On 2023/6/14 17:27, Chunhai Guo wrote:
Hi Jaegeuk,
Could you please help to confirm if this patch has been merged? I cannot
see the patch in the dev-test or dev branch.
Thanks.
On 2023/5/24 10:42, 郭纯海 wrote:
find_fsync_inode() detect the looped node chain by comparing the loop
counter with free blocks. While it may take tens of seconds to quit when
the free blocks are large enough. We can use Floyd's cycle detection
algorithm to make the detection more efficient, and fix the issue by
filling a NULL address in the last node of the chain.
Below is the log we encounter on a 256GB UFS storage and it takes about
25 seconds to detect looped node chain. After changing the algorithm, it
takes about 20ms to finish the same job.
[ 10.822904] fsck.f2fs: Info: version timestamp cur: 17, prev: 430
[ 10.822949] fsck.f2fs: [update_superblock: 762] Info: Done to
update superblock
[ 10.822953] fsck.f2fs: Info: superblock features = 1499 :
encrypt verity extra_attr project_quota quota_ino casefold
[ 10.822956] fsck.f2fs: Info: superblock encrypt level = 0, salt =
[ 10.822960] fsck.f2fs: Info: total FS sectors = 59249811 (231444
MB)
[ 35.852827] fsck.f2fs:detect looped node chain,
blkaddr:1114802, next:1114803
[ 35.852842] fsck.f2fs: [f2fs_do_mount:3846] record_fsync_data
failed
[ 35.856106] fsck.f2fs: fsck.f2fs terminated by exit(255)
Signed-off-by: Chunhai Guo
---
v7 -> v8 : Reformat the code to reduce indention.
v6 -> v7 : Correct logic error to handle is_detecting return by
find_node_blk_fast().
v5 -> v6 : Simplify the code by removing unnecessary retry logic.
v4 -> v5 : Use IS_INODE() to make the code more clear.
v3 -> v4 : Set c.bug_on with ASSERT_MSG() when issue is detected and fix
it only if c.fix_on is 1.
v2 -> v3 : Write inode with write_inode() to avoid chksum being broken.
v1 -> v2 : Fix looped node chain directly after it is detected.
---
fsck/mount.c | 127 +--
1 file changed, 112 insertions(+), 15 deletions(-)
diff --git a/fsck/mount.c b/fsck/mount.c
index 4c7488840c7c..9d6a222a055c 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -3518,22 +3518,90 @@ static void destroy_fsync_dnodes(struct list_head *head)
del_fsync_inode(entry);
}
+static int find_node_blk_fast(struct f2fs_sb_info *sbi, block_t *blkaddr_fast,
+ struct f2fs_node *node_blk_fast, bool *is_detecting)
+{
+ int i, err;
+
+ for (i = 0; i < 2; i++) {
+ if (!f2fs_is_valid_blkaddr(sbi, *blkaddr_fast, META_POR)) {
+ *is_detecting = false;
+ return 0;
+ }
+
+ err = dev_read_block(node_blk_fast, *blkaddr_fast);
+ if (err)
+ return err;
+
+ if (!is_recoverable_dnode(sbi, node_blk_fast)) {
+ *is_detecting = false;
+ return 0;
+ }
+
+ *blkaddr_fast = next_blkaddr_of_node(node_blk_fast);
+ }
+
+ return 0;
+}
+
+static int loop_node_chain_fix(struct f2fs_sb_info *sbi,
+ block_t blkaddr_fast, struct f2fs_node *node_blk_fast,
+ block_t blkaddr, struct f2fs_node *node_blk)
+{
+ block_t blkaddr_entry, blkaddr_tmp;
+ int err;
+
+ /* find the entry point of the looped node chain */
+ while (blkaddr_fast != blkaddr) {
+ err = dev_read_block(node_blk_fast, blkaddr_fast);
+ if (err)
+ return err;
+ blkaddr_fast = next_blkaddr_of_node(node_blk_fast);
+
+ err = dev_read_block(node_blk, blkaddr);
+ if (err)
+ return err;
+ blkaddr = next_blkaddr_of_node(node_blk);
+ }
+ blkaddr_entry = blkaddr;
+
+ /* find the last node of the chain */
+ do {
+ blkaddr_tmp = blkaddr;
+ err = dev_read_block(node_blk, blkaddr);
+ if (err)
+ return err;
+ blkaddr = next_blkaddr_of_node(node_blk);
+ } while (blkaddr != blkaddr_entry);
+
+ /* fix the blkaddr of last node with NULL_ADDR. */
+ node_blk->footer.next_blkaddr = NULL_ADDR;
+ if (IS_INODE(node_blk))
+ err = write_inode(node_blk, blkaddr_tmp);
+ else
+ err = dev_write_block(node_blk, blkaddr_tmp);
+ if (!err)
+ FIX_MSG("Fix looped node chain on blkaddr %u\n",
+ blkaddr_tmp);
+ return err;
+}
+
static int find_fsync_inode(struct f2fs_sb_info *sbi, struct list_head
*head)
{
struct
[f2fs-dev] [PATCH] fsck.f2fs: refactor looped node chain detetected logic for cleanup
Refactor looped node chain detected logic for cleanup as kernel does.
Suggested-by: Chao Yu
Signed-off-by: Chunhai Guo
---
fsck/mount.c | 115 ---
1 file changed, 55 insertions(+), 60 deletions(-)
diff --git a/fsck/mount.c b/fsck/mount.c
index e5ffb2602b9e..b4eb9ffeb685 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -3492,32 +3492,6 @@ static void destroy_fsync_dnodes(struct list_head *head)
del_fsync_inode(entry);
}
-static int find_node_blk_fast(struct f2fs_sb_info *sbi, block_t *blkaddr_fast,
- struct f2fs_node *node_blk_fast, bool *is_detecting)
-{
- int i, err;
-
- for (i = 0; i < 2; i++) {
- if (!f2fs_is_valid_blkaddr(sbi, *blkaddr_fast, META_POR)) {
- *is_detecting = false;
- return 0;
- }
-
- err = dev_read_block(node_blk_fast, *blkaddr_fast);
- if (err)
- return err;
-
- if (!is_recoverable_dnode(sbi, node_blk_fast)) {
- *is_detecting = false;
- return 0;
- }
-
- *blkaddr_fast = next_blkaddr_of_node(node_blk_fast);
- }
-
- return 0;
-}
-
static int loop_node_chain_fix(block_t blkaddr_fast,
struct f2fs_node *node_blk_fast,
block_t blkaddr, struct f2fs_node *node_blk)
@@ -3560,6 +3534,58 @@ static int loop_node_chain_fix(block_t blkaddr_fast,
return err;
}
+/* Detect looped node chain with Floyd's cycle detection algorithm. */
+static int sanity_check_node_chain(struct f2fs_sb_info *sbi,
+ block_t *blkaddr_fast, struct f2fs_node *node_blk_fast,
+ block_t blkaddr, struct f2fs_node *node_blk,
+ bool *is_detecting)
+{
+ int i, err;
+
+ if (!*is_detecting)
+ return 0;
+
+ for (i = 0; i < 2; i++) {
+ if (!f2fs_is_valid_blkaddr(sbi, *blkaddr_fast, META_POR)) {
+ *is_detecting = false;
+ return 0;
+ }
+
+ err = dev_read_block(node_blk_fast, *blkaddr_fast);
+ if (err)
+ return err;
+
+ if (!is_recoverable_dnode(sbi, node_blk_fast)) {
+ *is_detecting = false;
+ return 0;
+ }
+
+ *blkaddr_fast = next_blkaddr_of_node(node_blk_fast);
+ }
+
+ if (*blkaddr_fast != blkaddr)
+ return 0;
+
+ ASSERT_MSG("\tdetect looped node chain, blkaddr:%u\n", blkaddr);
+ if (!c.fix_on)
+ return -1;
+
+ err = loop_node_chain_fix(NEXT_FREE_BLKADDR(sbi,
+ CURSEG_I(sbi, CURSEG_WARM_NODE)),
+ node_blk_fast, blkaddr, node_blk);
+ if (err)
+ return err;
+
+ /* Since we call get_fsync_inode() to ensure there are no
+* duplicate inodes in the inode_list even if there are
+* duplicate blkaddr, we can continue running after fixing the
+* looped node chain.
+*/
+ *is_detecting = false;
+
+ return 0;
+}
+
static int find_fsync_inode(struct f2fs_sb_info *sbi, struct list_head *head)
{
struct curseg_info *curseg;
@@ -3608,42 +3634,11 @@ static int find_fsync_inode(struct f2fs_sb_info *sbi,
struct list_head *head)
next:
blkaddr = next_blkaddr_of_node(node_blk);
- /* Below we will detect looped node chain with Floyd's cycle
-* detection algorithm.
-*/
- if (!is_detecting)
- continue;
-
- err = find_node_blk_fast(sbi, _fast,
- node_blk_fast, _detecting);
+ err = sanity_check_node_chain(sbi, _fast,
+ node_blk_fast, blkaddr, node_blk,
+ _detecting);
if (err)
break;
-
- if (!is_detecting)
- continue;
-
- if (blkaddr_fast != blkaddr)
- continue;
-
- ASSERT_MSG("\tdetect looped node chain, blkaddr:%u\n",
- blkaddr);
-
- if (!c.fix_on) {
- err = -1;
- break;
- }
-
- err = loop_node_chain_fix(NEXT_FREE_BLKADDR(sbi, curseg),
- node_blk_fast, blkaddr, node_blk);
- if (err)
- break;
-
- /* Since we call get_fsync_inode() to ensure there are no
-* duplicate inodes in the inode_list even if there are
-* duplicate blkaddr, we can continue running after fixing the
-* looped node chain.
-*/
- is_detecting = false;
}
