linux-kernel - Re: [PATCH 11/14] fs: add a ->sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <47527262-c79b-43da-ad72-f52474c2cf30@nvidia.com>
Date: Mon, 17 Nov 2025 07:50:38 +0000
From: Chaitanya Kulkarni <chaitanyak@...dia.com>
To: Christoph Hellwig <hch@....de>, Christian Brauner <brauner@...nel.org>
CC: 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-kernel@...r.kernel.org>,
	"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
	"gfs2@...ts.linux.dev" <gfs2@...ts.linux.dev>, "io-uring@...r.kernel.org"
	<io-uring@...r.kernel.org>, "devel@...ts.orangefs.org"
	<devel@...ts.orangefs.org>, "linux-unionfs@...r.kernel.org"
	<linux-unionfs@...r.kernel.org>, "linux-mtd@...ts.infradead.org"
	<linux-mtd@...ts.infradead.org>, "linux-xfs@...r.kernel.org"
	<linux-xfs@...r.kernel.org>, "linux-nfs@...r.kernel.org"
	<linux-nfs@...r.kernel.org>
Subject: Re: [PATCH 11/14] fs: add a ->sync_lazytime method

On 11/13/25 22:26, Christoph Hellwig wrote:
> Allow the file system to explicitly implement lazytime syncing instead
> of pigging back on generic inode dirtying.  This allows to simplify
> the XFS implementation and prepares for non-blocking lazytime timestamp
> updates.
>
> Signed-off-by: Christoph Hellwig<hch@....de>
> ---
>   Documentation/filesystems/locking.rst |  2 ++
>   Documentation/filesystems/vfs.rst     |  6 ++++++
>   fs/fs-writeback.c                     | 13 +++++++++++--
>   include/linux/fs.h                    |  1 +
>   4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> index 77704fde9845..9b2f14ada8cd 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -81,6 +81,7 @@ prototypes::
>   	ssize_t (*listxattr) (struct dentry *, char *, size_t);
>   	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start, u64 len);
>   	void (*update_time)(struct inode *, struct timespec *, int);
> +	void (*sync_lazytime)(struct inode *inode);
>   	int (*atomic_open)(struct inode *, struct dentry *,
>   				struct file *, unsigned open_flag,
>   				umode_t create_mode);
> @@ -117,6 +118,7 @@ getattr:	no
>   listxattr:	no
>   fiemap:		no
>   update_time:	no
> +sync_lazytime:	no
>   atomic_open:	shared (exclusive if O_CREAT is set in open flags)
>   tmpfile:	no
>   fileattr_get:	no or exclusive
> diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
> index 4f13b01e42eb..ff59760daae2 100644
> --- a/Documentation/filesystems/vfs.rst
> +++ b/Documentation/filesystems/vfs.rst
> @@ -486,6 +486,7 @@ As of kernel 2.6.22, the following members are defined:
>   		int (*getattr) (struct mnt_idmap *, const struct path *, struct kstat *, u32, unsigned int);
>   		ssize_t (*listxattr) (struct dentry *, char *, size_t);
>   		void (*update_time)(struct inode *, struct timespec *, int);
> +		void (*sync_lazytime)(struct inode *inode);
>   		int (*atomic_open)(struct inode *, struct dentry *, struct file *,
>   				   unsigned open_flag, umode_t create_mode);
>   		int (*tmpfile) (struct mnt_idmap *, struct inode *, struct file *, umode_t);
> @@ -642,6 +643,11 @@ otherwise noted.
>   	an inode.  If this is not defined the VFS will update the inode
>   	itself and call mark_inode_dirty_sync.
>   
> +``sync_lazytime``:
> +	called by the writeback code to update the lazy time stamps to
> +	regular time stamp updates that get syncing into the on-disk
> +	inode.
> +
>   ``atomic_open``
>   	called on the last component of an open.  Using this optional
>   	method the filesystem can look up, possibly create and open the
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index ae6d1f1ccc71..7245f547416f 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1699,7 +1699,10 @@ bool sync_lazytime(struct inode *inode)
>   		return false;
>   
>   	trace_writeback_lazytime(inode);
> -	mark_inode_dirty_sync(inode);
> +	if (inode->i_op->sync_lazytime)
> +		inode->i_op->sync_lazytime(inode);
> +	else
> +		mark_inode_dirty_sync(inode);
>   	return false;
>   }
>   
> @@ -2547,6 +2550,8 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>   	trace_writeback_mark_inode_dirty(inode, flags);
>   
>   	if (flags & I_DIRTY_INODE) {
> +		bool was_dirty_time = true;
> +
>   		/*
>   		 * Inode timestamp update will piggback on this dirtying.
>   		 * We tell ->dirty_inode callback that timestamps need to
> @@ -2557,6 +2562,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
>   			if (inode->i_state & I_DIRTY_TIME) {
>   				inode->i_state &= ~I_DIRTY_TIME;
>   				flags |= I_DIRTY_TIME;
> +				was_dirty_time = true;
>   			}
>   			spin_unlock(&inode->i_lock);
>   		}

was_dirty_time is initialized it to true and above true again,
perhaps it should be initialized it to false, so it will be only set to
true when inode->i_state &= I_DIRTY_TIME; ?

if was_dirty_time always set to true at the time of initialization then
below check will always call as long as callback is set
inode->i_op->sync_lazytime(), irrespective of
inode->i_state &= I_DIRTY_TIME; assignment.


-ck