[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <j47ue7pzvtbg76hs5z7wov2ftjh2nnr4xxsslliiqjks5cmpwf@vz275nqw2bqb>
Date: Wed, 17 Dec 2025 13:23:43 +0100
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@....de>
Cc: Christian Brauner <brauner@...nel.org>,
Al Viro <viro@...iv.linux.org.uk>, David Sterba <dsterba@...e.com>, Jan Kara <jack@...e.cz>,
Mike Marshall <hubcap@...ibond.com>, Martin Brandenburg <martin@...ibond.com>,
Carlos Maiolino <cem@...nel.org>, Stefan Roesch <shr@...com>, Jeff Layton <jlayton@...nel.org>,
linux-kernel@...r.kernel.org, linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
gfs2@...ts.linux.dev, io-uring@...r.kernel.org, devel@...ts.orangefs.org,
linux-unionfs@...r.kernel.org, linux-mtd@...ts.infradead.org, linux-xfs@...r.kernel.org,
linux-nfs@...r.kernel.org
Subject: Re: [PATCH 05/10] fs: allow error returns from
inode_update_timestamps
On Wed 17-12-25 07:09:38, Christoph Hellwig wrote:
> Change flags to a by reference argument so that it can be updated so that
> the return value can be used for error returns. This will be used to
> implement non-blocking timestamp updates.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> Reviewed-by: Jeff Layton <jlayton@...nel.org>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/btrfs/inode.c | 8 +++++---
> fs/inode.c | 24 ++++++++++++++++--------
> fs/nfs/inode.c | 4 ++--
> fs/orangefs/inode.c | 5 ++++-
> fs/ubifs/file.c | 2 +-
> include/linux/fs.h | 2 +-
> 6 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 317db7d10a21..3ca8d294770e 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6349,13 +6349,15 @@ static int btrfs_dirty_inode(struct btrfs_inode *inode)
> static int btrfs_update_time(struct inode *inode, int flags)
> {
> struct btrfs_root *root = BTRFS_I(inode)->root;
> - bool dirty;
> + int error;
>
> if (btrfs_root_readonly(root))
> return -EROFS;
>
> - dirty = inode_update_timestamps(inode, flags);
> - return dirty ? btrfs_dirty_inode(BTRFS_I(inode)) : 0;
> + error = inode_update_timestamps(inode, &flags);
> + if (error || !flags)
> + return error;
> + return btrfs_dirty_inode(BTRFS_I(inode));
> }
>
> /*
> diff --git a/fs/inode.c b/fs/inode.c
> index 17ecb7bb5067..2c0d69f7fd01 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2095,14 +2095,18 @@ static bool relatime_need_update(struct vfsmount *mnt, struct inode *inode,
> * attempt to update all three of them. S_ATIME updates can be handled
> * independently of the rest.
> *
> - * Returns a set of S_* flags indicating which values changed.
> + * Updates @flags to contain the S_* flags which actually need changing. This
> + * can drop flags from the input when they don't need an update, or can add
> + * S_VERSION when the version needs to be bumped.
> + *
> + * Returns 0 or a negative errno.
> */
> -int inode_update_timestamps(struct inode *inode, int flags)
> +int inode_update_timestamps(struct inode *inode, int *flags)
> {
> int updated = 0;
> struct timespec64 now;
>
> - if (flags & (S_MTIME|S_CTIME|S_VERSION)) {
> + if (*flags & (S_MTIME | S_CTIME | S_VERSION)) {
> struct timespec64 ctime = inode_get_ctime(inode);
> struct timespec64 mtime = inode_get_mtime(inode);
>
> @@ -2119,7 +2123,7 @@ int inode_update_timestamps(struct inode *inode, int flags)
> now = current_time(inode);
> }
>
> - if (flags & S_ATIME) {
> + if (*flags & S_ATIME) {
> struct timespec64 atime = inode_get_atime(inode);
>
> if (!timespec64_equal(&now, &atime)) {
> @@ -2127,7 +2131,9 @@ int inode_update_timestamps(struct inode *inode, int flags)
> updated |= S_ATIME;
> }
> }
> - return updated;
> +
> + *flags = updated;
> + return 0;
> }
> EXPORT_SYMBOL(inode_update_timestamps);
>
> @@ -2145,10 +2151,12 @@ EXPORT_SYMBOL(inode_update_timestamps);
> */
> int generic_update_time(struct inode *inode, int flags)
> {
> - flags = inode_update_timestamps(inode, flags);
> - if (flags)
> + int error;
> +
> + error = inode_update_timestamps(inode, &flags);
> + if (!error && flags)
> mark_inode_dirty_time(inode, flags);
> - return 0;
> + return error;
> }
> EXPORT_SYMBOL(generic_update_time);
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 84049f3cd340..221816524c66 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -671,8 +671,8 @@ static void nfs_set_timestamps_to_ts(struct inode *inode, struct iattr *attr)
>
> static void nfs_update_timestamps(struct inode *inode, unsigned int ia_valid)
> {
> - enum file_time_flags time_flags = 0;
> unsigned int cache_flags = 0;
> + int time_flags = 0;
>
> if (ia_valid & ATTR_MTIME) {
> time_flags |= S_MTIME | S_CTIME;
> @@ -682,7 +682,7 @@ static void nfs_update_timestamps(struct inode *inode, unsigned int ia_valid)
> time_flags |= S_ATIME;
> cache_flags |= NFS_INO_INVALID_ATIME;
> }
> - inode_update_timestamps(inode, time_flags);
> + inode_update_timestamps(inode, &time_flags);
> NFS_I(inode)->cache_validity &= ~cache_flags;
> }
>
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index d7275990ffa4..3b58f31bd54f 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -875,11 +875,14 @@ int orangefs_permission(struct mnt_idmap *idmap,
> int orangefs_update_time(struct inode *inode, int flags)
> {
> struct iattr iattr;
> + int error;
>
> gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_update_time: %pU\n",
> get_khandle_from_ino(inode));
>
> - flags = inode_update_timestamps(inode, flags);
> + error = inode_update_timestamps(inode, &flags);
> + if (error || !flags)
> + return error;
>
> memset(&iattr, 0, sizeof iattr);
> if (flags & S_ATIME)
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index ec1bb9f43acc..71540644a931 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1387,7 +1387,7 @@ int ubifs_update_time(struct inode *inode, int flags)
> return err;
>
> mutex_lock(&ui->ui_mutex);
> - inode_update_timestamps(inode, flags);
> + inode_update_timestamps(inode, &flags);
> release = ui->dirty;
> __mark_inode_dirty(inode, I_DIRTY_SYNC);
> mutex_unlock(&ui->ui_mutex);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 66d3d18cf4e3..75d5f38b08c9 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2399,7 +2399,7 @@ static inline void super_set_sysfs_name_generic(struct super_block *sb, const ch
> extern void ihold(struct inode * inode);
> extern void iput(struct inode *);
> void iput_not_last(struct inode *);
> -int inode_update_timestamps(struct inode *inode, int flags);
> +int inode_update_timestamps(struct inode *inode, int *flags);
> int generic_update_time(struct inode *inode, int flags);
>
> /* /sys/fs */
> --
> 2.47.3
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists