| Message ID | 32a87813b58c1ddc3ae4d769cd2b667901621f6a.1634231213.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update device mod time fixes | expand |
On Thu, Oct 14, 2021 at 01:11:00PM -0400, Josef Bacik wrote: > If you already have an inode and need to update the time on the inode > there is no way to do this properly. Export this helper to allow file > systems to update time on the inode so the appropriate handler is > called, either ->update_time or generic_update_time. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> I'd like to get ack from Christoph, though it's a simple change it's still in another subsystem. > --- > fs/inode.c | 7 ++++--- > include/linux/fs.h | 2 ++ > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/fs/inode.c b/fs/inode.c > index ed0cab8a32db..9abc88d7959c 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -1782,12 +1782,13 @@ EXPORT_SYMBOL(generic_update_time); > * This does the actual work of updating an inodes time or version. Must have > * had called mnt_want_write() before calling this. > */ > -static int update_time(struct inode *inode, struct timespec64 *time, int flags) > +int inode_update_time(struct inode *inode, struct timespec64 *time, int flags) > { > if (inode->i_op->update_time) > return inode->i_op->update_time(inode, time, flags); > return generic_update_time(inode, time, flags); > } > +EXPORT_SYMBOL(inode_update_time); > > /** > * atime_needs_update - update the access time > @@ -1857,7 +1858,7 @@ void touch_atime(const struct path *path) > * of the fs read only, e.g. subvolumes in Btrfs. > */ > now = current_time(inode); > - update_time(inode, &now, S_ATIME); > + inode_update_time(inode, &now, S_ATIME); > __mnt_drop_write(mnt); > skip_update: > sb_end_write(inode->i_sb); > @@ -2002,7 +2003,7 @@ int file_update_time(struct file *file) > if (__mnt_want_write_file(file)) > return 0; > > - ret = update_time(inode, &now, sync_it); > + ret = inode_update_time(inode, &now, sync_it); > __mnt_drop_write_file(file); > > return ret; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index e7a633353fd2..20e2fe398ab6 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2498,6 +2498,8 @@ enum file_time_flags { > > extern bool atime_needs_update(const struct path *, struct inode *); > extern void touch_atime(const struct path *); > +extern int inode_update_time(struct inode *inode, struct timespec64 *time, > + int flags); As was pointed out in the past for similar patches, the 'extern' can be dropped, so I'll that.
On Thu, Oct 21, 2021 at 06:38:17PM +0200, David Sterba wrote: > On Thu, Oct 14, 2021 at 01:11:00PM -0400, Josef Bacik wrote: > > If you already have an inode and need to update the time on the inode > > there is no way to do this properly. Export this helper to allow file > > systems to update time on the inode so the appropriate handler is > > called, either ->update_time or generic_update_time. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > I'd like to get ack from Christoph, though it's a simple change it's > still in another subsystem. Not a big fan, but compared to the other options it seems like the least bad option. That being said I'm not the VFS maintainer anyway.
On Mon, Oct 25, 2021 at 09:45:09AM +0200, Christoph Hellwig wrote: > On Thu, Oct 21, 2021 at 06:38:17PM +0200, David Sterba wrote: > > On Thu, Oct 14, 2021 at 01:11:00PM -0400, Josef Bacik wrote: > > > If you already have an inode and need to update the time on the inode > > > there is no way to do this properly. Export this helper to allow file > > > systems to update time on the inode so the appropriate handler is > > > called, either ->update_time or generic_update_time. > > > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > > > I'd like to get ack from Christoph, though it's a simple change it's > > still in another subsystem. > > Not a big fan, but compared to the other options it seems like the > least bad option. Thanks, given that it fixes the time update problem and is otherwise quite unintrusive it should be ok. > That being said I'm not the VFS maintainer anyway. Well yeah, but you reviewed or was involved in some other similar changes, so I take it as a confirmation that we're not abusing some VFS internal. But I'm adding Al Viro to CC anyway.
diff --git a/fs/inode.c b/fs/inode.c index ed0cab8a32db..9abc88d7959c 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1782,12 +1782,13 @@ EXPORT_SYMBOL(generic_update_time); * This does the actual work of updating an inodes time or version. Must have * had called mnt_want_write() before calling this. */ -static int update_time(struct inode *inode, struct timespec64 *time, int flags) +int inode_update_time(struct inode *inode, struct timespec64 *time, int flags) { if (inode->i_op->update_time) return inode->i_op->update_time(inode, time, flags); return generic_update_time(inode, time, flags); } +EXPORT_SYMBOL(inode_update_time); /** * atime_needs_update - update the access time @@ -1857,7 +1858,7 @@ void touch_atime(const struct path *path) * of the fs read only, e.g. subvolumes in Btrfs. */ now = current_time(inode); - update_time(inode, &now, S_ATIME); + inode_update_time(inode, &now, S_ATIME); __mnt_drop_write(mnt); skip_update: sb_end_write(inode->i_sb); @@ -2002,7 +2003,7 @@ int file_update_time(struct file *file) if (__mnt_want_write_file(file)) return 0; - ret = update_time(inode, &now, sync_it); + ret = inode_update_time(inode, &now, sync_it); __mnt_drop_write_file(file); return ret; diff --git a/include/linux/fs.h b/include/linux/fs.h index e7a633353fd2..20e2fe398ab6 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2498,6 +2498,8 @@ enum file_time_flags { extern bool atime_needs_update(const struct path *, struct inode *); extern void touch_atime(const struct path *); +extern int inode_update_time(struct inode *inode, struct timespec64 *time, + int flags); static inline void file_accessed(struct file *file) { if (!(file->f_flags & O_NOATIME))
If you already have an inode and need to update the time on the inode there is no way to do this properly. Export this helper to allow file systems to update time on the inode so the appropriate handler is called, either ->update_time or generic_update_time. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/inode.c | 7 ++++--- include/linux/fs.h | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-)