All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Christian Brauner <brauner@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	David Sterba <dsterba@suse.com>, Jan Kara <jack@suse.cz>,
	Mike Marshall <hubcap@omnibond.com>,
	Martin Brandenburg <martin@omnibond.com>,
	Carlos Maiolino <cem@kernel.org>, Stefan Roesch <shr@fb.com>,
	Jeff Layton <jlayton@kernel.org>,
	linux-kernel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	gfs2@lists.linux.dev, io-uring@vger.kernel.org,
	devel@lists.orangefs.org, linux-unionfs@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-xfs@vger.kernel.org,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH 14/14] xfs: enable non-blocking timestamp updates
Date: Sun, 16 Nov 2025 19:23:11 +1100	[thread overview]
Message-ID: <aRmJ728evgFnBLhn@dread.disaster.area> (raw)
In-Reply-To: <20251114062642.1524837-15-hch@lst.de>

On Fri, Nov 14, 2025 at 07:26:17AM +0100, Christoph Hellwig wrote:
> The lazytime path using generic_update_time can never block in XFS
> because there is no ->dirty_inode method that could block.  Allow
> non-blocking timestamp updates for this case.
> 
> Fixes: 66fa3cedf16a ("fs: Add async write file modification handling.")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_iops.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index bd0b7e81f6ab..3d7b89ffacde 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1195,9 +1195,6 @@ xfs_vn_update_time(
>  
>  	trace_xfs_update_time(ip);
>  
> -	if (flags & S_NOWAIT)
> -		return -EAGAIN;
> -
>  	if (inode->i_sb->s_flags & SB_LAZYTIME) {
>  		if (!((flags & S_VERSION) &&
>  		      inode_maybe_inc_iversion(inode, false)))
> @@ -1207,6 +1204,9 @@ xfs_vn_update_time(
>  		log_flags |= XFS_ILOG_CORE;
>  	}
>  
> +	if (flags & S_NOWAIT)
> +		return -EAGAIN;
> +
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp);
>  	if (error)
>  		return error;

Not sure this is correct - this can now bump iversion and then
return -EAGAIN. That means S_VERSION likely won't be set on the
retry, and we'll go straight through the non-blocking path to
generic_update_time() and skip logging the iversion update....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Christian Brauner <brauner@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	David Sterba <dsterba@suse.com>, Jan Kara <jack@suse.cz>,
	Mike Marshall <hubcap@omnibond.com>,
	Martin Brandenburg <martin@omnibond.com>,
	Carlos Maiolino <cem@kernel.org>, Stefan Roesch <shr@fb.com>,
	Jeff Layton <jlayton@kernel.org>,
	linux-kernel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	gfs2@lists.linux.dev, io-uring@vger.kernel.org,
	devel@lists.orangefs.org, linux-unionfs@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-xfs@vger.kernel.org,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH 14/14] xfs: enable non-blocking timestamp updates
Date: Sun, 16 Nov 2025 19:23:11 +1100	[thread overview]
Message-ID: <aRmJ728evgFnBLhn@dread.disaster.area> (raw)
In-Reply-To: <20251114062642.1524837-15-hch@lst.de>

On Fri, Nov 14, 2025 at 07:26:17AM +0100, Christoph Hellwig wrote:
> The lazytime path using generic_update_time can never block in XFS
> because there is no ->dirty_inode method that could block.  Allow
> non-blocking timestamp updates for this case.
> 
> Fixes: 66fa3cedf16a ("fs: Add async write file modification handling.")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_iops.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index bd0b7e81f6ab..3d7b89ffacde 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1195,9 +1195,6 @@ xfs_vn_update_time(
>  
>  	trace_xfs_update_time(ip);
>  
> -	if (flags & S_NOWAIT)
> -		return -EAGAIN;
> -
>  	if (inode->i_sb->s_flags & SB_LAZYTIME) {
>  		if (!((flags & S_VERSION) &&
>  		      inode_maybe_inc_iversion(inode, false)))
> @@ -1207,6 +1204,9 @@ xfs_vn_update_time(
>  		log_flags |= XFS_ILOG_CORE;
>  	}
>  
> +	if (flags & S_NOWAIT)
> +		return -EAGAIN;
> +
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp);
>  	if (error)
>  		return error;

Not sure this is correct - this can now bump iversion and then
return -EAGAIN. That means S_VERSION likely won't be set on the
retry, and we'll go straight through the non-blocking path to
generic_update_time() and skip logging the iversion update....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2025-11-16  8:23 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14  6:26 re-enable IOCB_NOWAIT writes to files Christoph Hellwig
2025-11-14  6:26 ` Christoph Hellwig
2025-11-14  6:26 ` [PATCH 01/14] fs: refactor file timestamp update logic Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  6:32   ` Chaitanya Kulkarni
2025-11-17  6:32     ` Chaitanya Kulkarni
2025-11-24 11:51   ` Jan Kara
2025-11-24 11:51     ` Jan Kara
2025-11-14  6:26 ` [PATCH 02/14] fs: lift the FMODE_NOCMTIME check into file_update_time_flags Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  6:38   ` Chaitanya Kulkarni
2025-11-17  6:38     ` Chaitanya Kulkarni
2025-11-24 11:54   ` Jan Kara
2025-11-24 11:54     ` Jan Kara
2025-11-14  6:26 ` [PATCH 03/14] fs: export vfs_utimes Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  6:38   ` Chaitanya Kulkarni
2025-11-17  6:38     ` Chaitanya Kulkarni
2025-11-24 11:55   ` Jan Kara
2025-11-24 11:55     ` Jan Kara
2025-11-14  6:26 ` [PATCH 04/14] btrfs: use vfs_utimes to update file timestamps Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-14  6:26 ` [PATCH 05/14] fs: remove inode_update_time Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  6:59   ` Chaitanya Kulkarni
2025-11-17  6:59     ` Chaitanya Kulkarni
2025-11-19  6:25     ` Christoph Hellwig
2025-11-19  6:25       ` Christoph Hellwig
2025-11-24 13:06   ` Jan Kara
2025-11-24 13:06     ` Jan Kara
2025-11-14  6:26 ` [PATCH 06/14] organgefs: use inode_update_timestamps directly Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-14 14:06   ` Jeff Layton
2025-11-14 14:06     ` Jeff Layton
2025-11-14 15:26     ` Christoph Hellwig
2025-11-14 15:26       ` Christoph Hellwig
2025-11-24 13:09   ` Jan Kara
2025-11-24 13:09     ` Jan Kara
2025-11-14  6:26 ` [PATCH 07/14] fs: return a negative error from generic_update_time Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  7:07   ` Chaitanya Kulkarni
2025-11-17  7:07     ` Chaitanya Kulkarni
2025-11-24 13:10   ` Jan Kara
2025-11-24 13:10     ` Jan Kara
2025-11-14  6:26 ` [PATCH 08/14] fs: exit early in generic_update_time when there is no work Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-24 13:11   ` Jan Kara
2025-11-24 13:11     ` Jan Kara
2025-11-14  6:26 ` [PATCH 09/14] fs: factor out a mark_inode_dirty_time helper Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  7:13   ` Chaitanya Kulkarni
2025-11-17  7:13     ` Chaitanya Kulkarni
2025-11-24 13:22   ` Jan Kara
2025-11-24 13:22     ` Jan Kara
2025-11-24 14:07     ` Christoph Hellwig
2025-11-24 14:07       ` Christoph Hellwig
2025-11-14  6:26 ` [PATCH 10/14] fs: factor out a sync_lazytime helper Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  7:38   ` Chaitanya Kulkarni
2025-11-17  7:38     ` Chaitanya Kulkarni
2025-11-19  6:28     ` Christoph Hellwig
2025-11-19  6:28       ` Christoph Hellwig
2025-11-24 13:31   ` Jan Kara
2025-11-24 13:31     ` Jan Kara
2025-11-24 14:09     ` Christoph Hellwig
2025-11-24 14:09       ` Christoph Hellwig
2025-11-24 17:36       ` Jan Kara
2025-11-24 17:36         ` Jan Kara
2025-11-14  6:26 ` [PATCH 11/14] fs: add a ->sync_lazytime method Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-17  7:50   ` Chaitanya Kulkarni
2025-11-17  7:50     ` Chaitanya Kulkarni
2025-11-14  6:26 ` [PATCH 12/14] fs: add support for non-blocking timestamp updates Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-14  6:26 ` [PATCH 13/14] xfs: implement ->sync_lazytime Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-14  6:26 ` [PATCH 14/14] xfs: enable non-blocking timestamp updates Christoph Hellwig
2025-11-14  6:26   ` Christoph Hellwig
2025-11-14 15:30   ` Christoph Hellwig
2025-11-14 15:30     ` Christoph Hellwig
2025-11-16  8:23   ` Dave Chinner [this message]
2025-11-16  8:23     ` Dave Chinner
2025-11-19  7:29     ` Christoph Hellwig
2025-11-19  7:29       ` Christoph Hellwig
2025-11-14 14:04 ` re-enable IOCB_NOWAIT writes to files Jeff Layton
2025-11-14 14:04   ` Jeff Layton
2025-11-14 15:28   ` Christoph Hellwig
2025-11-14 15:28     ` Christoph Hellwig
2025-11-14 19:47     ` Jeff Layton
2025-11-14 19:47       ` Jeff Layton
2025-11-14 17:01   ` Darrick J. Wong
2025-11-14 17:01     ` Darrick J. Wong
2025-11-14 17:21     ` Jeff Layton
2025-11-14 17:21       ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRmJ728evgFnBLhn@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=devel@lists.orangefs.org \
    --cc=dsterba@suse.com \
    --cc=gfs2@lists.linux.dev \
    --cc=hch@lst.de \
    --cc=hubcap@omnibond.com \
    --cc=io-uring@vger.kernel.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin@omnibond.com \
    --cc=shr@fb.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.