All of lore.kernel.org
 help / color / mirror / Atom feed
From: Horst Birthelmer <horst@birthelmer.de>
To: Jingbo Xu <jefflexu@linux.alibaba.com>
Cc: miklos@szeredi.hu, fuse-devel@lists.linux.dev,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fuse: don't bump attr_version for async direct read completion
Date: Mon, 6 Jul 2026 10:38:53 +0200	[thread overview]
Message-ID: <aktpOW1LWNkn-jlE@fedora.fritz.box> (raw)
In-Reply-To: <20260706082931.99288-1-jefflexu@linux.alibaba.com>

On Mon, Jul 06, 2026 at 04:29:31PM +0800, Jingbo Xu wrote:
> The attr_version counter should only be incremented when cached
> attributes are actually modified (as documented in commit 1fb69e781729
> ("fuse: fix race between getattr and write")).  Async direct reads do not
> modify any cached inode attributes (size, mtime, ctime), so bumping
> attr_version in fuse_aio_complete() for reads is incorrect.
> 
> This unconditional bump causes a livelock when auto_inval_data is
> enabled together with writeback_cache: fuse_cache_read_iter() issues a
> FUSE_GETATTR before every read, but by the time the response arrives,
> an async DIO read completion has already incremented fi->attr_version
> past the snapshot taken before the request.  The GETATTR result is then
> discarded (attr_version race), fi->i_time is never refreshed, and every
> subsequent read triggers yet another GETATTR -- creating an infinite
> loop of useless round-trips.
> 
> Fix this by only bumping attr_version for write completions, consistent
> with the synchronous DIO write path (fuse_write_update_attr) and the
> cached write path.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> ---
>  fs/fuse/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index ceada75310b8..c41287e8bc15 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -688,7 +688,7 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
>  		struct address_space *mapping = io->iocb->ki_filp->f_mapping;
>  		ssize_t res = fuse_get_res_by_io(io);
>  
> -		if (res >= 0) {
> +		if (res >= 0 && io->write) {
>  			struct fuse_conn *fc = get_fuse_conn(inode);
>  			struct fuse_inode *fi = get_fuse_inode(inode);
>  
> -- 
> 2.19.1.6.gb485710b
> 
> 
LGTM

Reviewed-by: Horst Birthelmer <hbirthelmer@ddn.com>

      reply	other threads:[~2026-07-06  8:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06  8:29 [PATCH] fuse: don't bump attr_version for async direct read completion Jingbo Xu
2026-07-06  8:38 ` Horst Birthelmer [this message]

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=aktpOW1LWNkn-jlE@fedora.fritz.box \
    --to=horst@birthelmer.de \
    --cc=fuse-devel@lists.linux.dev \
    --cc=jefflexu@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /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.