| Message ID | 20240711-mgtime-v5-4-37bb5b465feb@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fs: multigrain timestamp redux | expand |
On Thu, Jul 11, 2024 at 07:08:08AM -0400, Jeff Layton wrote: > The setattr codepath is still using coarse-grained timestamps, even on > multigrain filesystems. To fix this, we need to fetch the timestamp for > ctime updates later, at the point where the assignment occurs in > setattr_copy. > > On a multigrain inode, ignore the ia_ctime in the attrs, and always > update the ctime to the current clock value. Update the atime and mtime > with the same value (if needed) unless they are being set to other > specific values, a'la utimes(). > > Note that we don't want to do this universally however, as some > filesystems (e.g. most networked fs) want to do an explicit update > elsewhere before updating the local inode. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> Makes sense to me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/attr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 46 insertions(+), 6 deletions(-) > > diff --git a/fs/attr.c b/fs/attr.c > index 825007d5cda4..e03ea6951864 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -271,6 +271,42 @@ int inode_newsize_ok(const struct inode *inode, loff_t offset) > } > EXPORT_SYMBOL(inode_newsize_ok); > > +/** > + * setattr_copy_mgtime - update timestamps for mgtime inodes > + * @inode: inode timestamps to be updated > + * @attr: attrs for the update > + * > + * With multigrain timestamps, we need to take more care to prevent races > + * when updating the ctime. Always update the ctime to the very latest > + * using the standard mechanism, and use that to populate the atime and > + * mtime appropriately (unless we're setting those to specific values). > + */ > +static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr) > +{ > + unsigned int ia_valid = attr->ia_valid; > + struct timespec64 now; > + > + /* > + * If the ctime isn't being updated then nothing else should be > + * either. > + */ > + if (!(ia_valid & ATTR_CTIME)) { > + WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME)); > + return; > + } > + > + now = inode_set_ctime_current(inode); > + if (ia_valid & ATTR_ATIME_SET) > + inode_set_atime_to_ts(inode, attr->ia_atime); > + else if (ia_valid & ATTR_ATIME) > + inode_set_atime_to_ts(inode, now); > + > + if (ia_valid & ATTR_MTIME_SET) > + inode_set_mtime_to_ts(inode, attr->ia_mtime); > + else if (ia_valid & ATTR_MTIME) > + inode_set_mtime_to_ts(inode, now); > +} > + > /** > * setattr_copy - copy simple metadata updates into the generic inode > * @idmap: idmap of the mount the inode was found from > @@ -303,12 +339,6 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode, > > i_uid_update(idmap, attr, inode); > i_gid_update(idmap, attr, inode); > - if (ia_valid & ATTR_ATIME) > - inode_set_atime_to_ts(inode, attr->ia_atime); > - if (ia_valid & ATTR_MTIME) > - inode_set_mtime_to_ts(inode, attr->ia_mtime); > - if (ia_valid & ATTR_CTIME) > - inode_set_ctime_to_ts(inode, attr->ia_ctime); > if (ia_valid & ATTR_MODE) { > umode_t mode = attr->ia_mode; > if (!in_group_or_capable(idmap, inode, > @@ -316,6 +346,16 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode, > mode &= ~S_ISGID; > inode->i_mode = mode; > } > + > + if (is_mgtime(inode)) > + return setattr_copy_mgtime(inode, attr); > + > + if (ia_valid & ATTR_ATIME) > + inode_set_atime_to_ts(inode, attr->ia_atime); > + if (ia_valid & ATTR_MTIME) > + inode_set_mtime_to_ts(inode, attr->ia_mtime); > + if (ia_valid & ATTR_CTIME) > + inode_set_ctime_to_ts(inode, attr->ia_ctime); > } > EXPORT_SYMBOL(setattr_copy); > > > -- > 2.45.2 > >
diff --git a/fs/attr.c b/fs/attr.c index 825007d5cda4..e03ea6951864 100644 --- a/fs/attr.c +++ b/fs/attr.c @@ -271,6 +271,42 @@ int inode_newsize_ok(const struct inode *inode, loff_t offset) } EXPORT_SYMBOL(inode_newsize_ok); +/** + * setattr_copy_mgtime - update timestamps for mgtime inodes + * @inode: inode timestamps to be updated + * @attr: attrs for the update + * + * With multigrain timestamps, we need to take more care to prevent races + * when updating the ctime. Always update the ctime to the very latest + * using the standard mechanism, and use that to populate the atime and + * mtime appropriately (unless we're setting those to specific values). + */ +static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr) +{ + unsigned int ia_valid = attr->ia_valid; + struct timespec64 now; + + /* + * If the ctime isn't being updated then nothing else should be + * either. + */ + if (!(ia_valid & ATTR_CTIME)) { + WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME)); + return; + } + + now = inode_set_ctime_current(inode); + if (ia_valid & ATTR_ATIME_SET) + inode_set_atime_to_ts(inode, attr->ia_atime); + else if (ia_valid & ATTR_ATIME) + inode_set_atime_to_ts(inode, now); + + if (ia_valid & ATTR_MTIME_SET) + inode_set_mtime_to_ts(inode, attr->ia_mtime); + else if (ia_valid & ATTR_MTIME) + inode_set_mtime_to_ts(inode, now); +} + /** * setattr_copy - copy simple metadata updates into the generic inode * @idmap: idmap of the mount the inode was found from @@ -303,12 +339,6 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode, i_uid_update(idmap, attr, inode); i_gid_update(idmap, attr, inode); - if (ia_valid & ATTR_ATIME) - inode_set_atime_to_ts(inode, attr->ia_atime); - if (ia_valid & ATTR_MTIME) - inode_set_mtime_to_ts(inode, attr->ia_mtime); - if (ia_valid & ATTR_CTIME) - inode_set_ctime_to_ts(inode, attr->ia_ctime); if (ia_valid & ATTR_MODE) { umode_t mode = attr->ia_mode; if (!in_group_or_capable(idmap, inode, @@ -316,6 +346,16 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode, mode &= ~S_ISGID; inode->i_mode = mode; } + + if (is_mgtime(inode)) + return setattr_copy_mgtime(inode, attr); + + if (ia_valid & ATTR_ATIME) + inode_set_atime_to_ts(inode, attr->ia_atime); + if (ia_valid & ATTR_MTIME) + inode_set_mtime_to_ts(inode, attr->ia_mtime); + if (ia_valid & ATTR_CTIME) + inode_set_ctime_to_ts(inode, attr->ia_ctime); } EXPORT_SYMBOL(setattr_copy);
The setattr codepath is still using coarse-grained timestamps, even on multigrain filesystems. To fix this, we need to fetch the timestamp for ctime updates later, at the point where the assignment occurs in setattr_copy. On a multigrain inode, ignore the ia_ctime in the attrs, and always update the ctime to the current clock value. Update the atime and mtime with the same value (if needed) unless they are being set to other specific values, a'la utimes(). Note that we don't want to do this universally however, as some filesystems (e.g. most networked fs) want to do an explicit update elsewhere before updating the local inode. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/attr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 6 deletions(-)