Re: [f2fs-dev] [PATCH v2] f2fs: fix to avoid racing in between read and OPU dio write

[Chao Yu]

On 2024/6/26 10:01, Zhiguo Niu wrote:
> Chao Yu <chao@xxxxxxxxxx> 于2024年6月25日周二 22:29写道：
> > If lfs mode is on, buffered read may race w/ OPU dio write as below,
> > it may cause buffered read hits unwritten data unexpectly, and for
> > dio read, the race condition exists as well.
> >
> > Thread A                        Thread B
> > - f2fs_file_write_iter
> >   - f2fs_dio_write_iter
> >    - __iomap_dio_rw
> >     - f2fs_iomap_begin
> >      - f2fs_map_blocks
> >       - __allocate_data_block
> >        - allocated blkaddr #x
> >         - iomap_dio_submit_bio
> >                                  - f2fs_file_read_iter
> >                                   - filemap_read
> >                                    - f2fs_read_data_folio
> >                                     - f2fs_mpage_readpages
> >                                      - f2fs_map_blocks
> >                                       : get blkaddr #x
> >                                      - f2fs_submit_read_bio
> >                                  IRQ
> >                                  - f2fs_read_end_io
> >                                   : read IO on blkaddr #x complete
> > IRQ
> > - iomap_dio_bio_end_io
> >   : direct write IO on blkaddr #x complete
> >
> > In LFS mode, if there is inflight dio, let's force read to buffered
> > IO, this policy won't cover all race cases, however it is a tradeoff
> > which avoids abusing lock around IO paths.
> >
> > Fixes: f847c699cff3 ("f2fs: allow out-place-update for direct IO in LFS mode")
> > Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
> > ---
> >   fs/f2fs/file.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 278573974db4..866f1a34e92b 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -882,6 +882,10 @@ static bool f2fs_force_buffered_io(struct inode *inode, int rw)
> >                  return true;
> >          if (is_sbi_flag_set(sbi, SBI_CP_DISABLED))
> >                  return true;
> > +       /* In LFS mode, if there is inflight dio, force read to buffered IO */
> > +       if (rw == READ && f2fs_lfs_mode(sbi) &&
> > +                       atomic_read(&inode->i_dio_count))
> > +               return false;
> Hi Chao,
> A little doubt:)，force “buffered IO” should return "true"?

Oops, too rush to send the patch...

> another want to confirm is, "thread B" in commit msg just doing buffer
> read, so this modification just cover direct read case？

Oh, the fix is incorrect, will look into it soon.

Thanks,

> thanks！
> >          return false;
> >   }
> > --
> > 2.40.1
> >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel