linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Joanne Koong <joannelkoong@gmail.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>, lu gu <giveme.gulu@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bernd Schubert <bernd@bsbernd.com>
Subject: Re: [PATCH 5.15] fuse: Fix race condition in writethrough path A race
Date: Wed, 15 Oct 2025 15:48:19 -0400	[thread overview]
Message-ID: <aO_6g9cG1IVvp--D@bfoster> (raw)
In-Reply-To: <CAJnrk1b=UMb9GrU0oiah986of_dgwLiRsZKvodwBoO1PSUaP7w@mail.gmail.com>

On Wed, Oct 15, 2025 at 10:19:15AM -0700, Joanne Koong wrote:
> On Wed, Oct 15, 2025 at 7:09 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, 15 Oct 2025 at 06:00, lu gu <giveme.gulu@gmail.com> wrote:
> > >
> > > >  Attaching a test patch, minimally tested.
> > > Since I only have a test environment for kernel 5.15, I ported this
> > > patch to the FUSE module in 5.15. I ran the previous LTP test cases
> > > more than ten times, and the data inconsistency issue did not reoccur.
> > > However, a deadlock occur. Below is the specific stack trace.
> >
> > This is does not reproduce for me on 6.17 even after running the test
> > for hours.  Without seeing your backport it is difficult to say
> > anything about the reason for the deadlock.
> >
> > Attaching an updated patch that takes care of i_wb initialization on
> > CONFIG_CGROUP_WRITEBACK=y.
> 
> I think now we'll also need to always set
> mapping_set_writeback_may_deadlock_on_reclaim(), eg
> 
> @@ -3125,8 +3128,7 @@ void fuse_init_file_inode(struct inode *inode,
> unsigned int flags)
> 
>         inode->i_fop = &fuse_file_operations;
>         inode->i_data.a_ops = &fuse_file_aops;
> -       if (fc->writeback_cache)
> -               mapping_set_writeback_may_deadlock_on_reclaim(&inode->i_data);
> +       mapping_set_writeback_may_deadlock_on_reclaim(&inode->i_data);
> 
> 
> Does this completely get rid of the race? There's a fair chance I'm
> wrong here but doesn't the race still happen if the read invalidation
> happens before the write grabs the folio lock? This is the scenario
> I'm thinking of:
> 
> Thread A (read):
> read, w/ auto inval and a outdated mtime triggers invalidate_inode_pages2()
> generic_file_read_iter() is called, which calls filemap_read() ->
> filemap_get_pages() -> triggers read_folio/readahead
> read_folio/readahead fetches data (stale) from the server, unlocks folios
> 
> Thread B (writethrough write):
> fuse_perform_write() -> fuse_fill_write_pages():
> grabs the folio lock and copies new write data to page cache, sets
> writeback flag and unlocks folio, sends request to server
> 
> Thread A (read):
> the read data that was fetched from the server gets copied to the page
> cache in filemap_read()
> overwrites the write data in the page cache with the stale data
> 
> Am i misanalyzing something in this sequence?
> 

Maybe I misread the description, but I think folios are locked across
read I/O, so I don't follow how we could race with readahead in this
way. Hm?

Brian

> Thanks,
> Joanne
> >
> > Thanks,
> > Miklos
> 


  reply	other threads:[~2025-10-15 19:44 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-09 11:06 [PATCH 5.15] fuse: Fix race condition in writethrough path A race guangming.zhao
2025-10-09 22:11 ` Joanne Koong
     [not found]   ` <CAFS-8+VcZn7WZgjV9pHz4c8DYHRdP0on6-er5fm9TZF9RAO0xQ@mail.gmail.com>
2025-10-10  6:25     ` lu gu
2025-10-10  8:46       ` Miklos Szeredi
2025-10-13 13:39         ` Miklos Szeredi
2025-10-13 17:44           ` Brian Foster
2025-10-13 18:23             ` Miklos Szeredi
2025-10-13 18:53               ` Brian Foster
2025-10-14  7:48                 ` Miklos Szeredi
2025-10-14 12:43                   ` Miklos Szeredi
2025-10-14 16:15                     ` Miklos Szeredi
2025-10-14 17:01                     ` Joanne Koong
2025-10-14 17:56                       ` Brian Foster
2025-10-15  3:59                         ` lu gu
2025-10-15 14:09                           ` Miklos Szeredi
2025-10-15 17:19                             ` Joanne Koong
2025-10-15 19:48                               ` Brian Foster [this message]
2025-10-15 20:28                                 ` Joanne Koong
2025-10-20 10:10                                   ` lu gu
2025-10-14 14:01                   ` Brian Foster
2025-10-14 16:10                     ` Miklos Szeredi
2025-10-14 16:15                       ` Bernd Schubert
2025-10-14 16:21                       ` Brian Foster
2025-10-13 20:16           ` Bernd Schubert
2025-10-13 20:27             ` Joanne Koong
2025-10-13 20:40               ` Bernd Schubert
2025-10-13 23:32                 ` Joanne Koong
2025-10-14  8:06             ` Miklos Szeredi
2025-10-13 23:43           ` Joanne Koong
2025-10-14  8:11             ` Miklos Szeredi
2025-10-14  9:36               ` lu gu

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=aO_6g9cG1IVvp--D@bfoster \
    --to=bfoster@redhat.com \
    --cc=bernd@bsbernd.com \
    --cc=giveme.gulu@gmail.com \
    --cc=joannelkoong@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).