On 10:42 30/06, David Sterba wrote: > On Mon, Jun 22, 2020 at 11:20:15AM -0500, Goldwyn Rodrigues wrote: > > From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> > > > > This is to parallelize direct writes within EOF or with direct I/O > > reads. This covers the race with truncate() accidentally increasing the > > filesize. > > Please describe the race condition in more detail and how the DIO/EOF > parallelization is supposed to work. > > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> > > --- > > fs/btrfs/file.c | 25 +++++++------------------ > > 1 file changed, 7 insertions(+), 18 deletions(-) > > > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > > index aa6be931620b..c446a4aeb867 100644 > > --- a/fs/btrfs/file.c > > +++ b/fs/btrfs/file.c > > @@ -1957,12 +1957,18 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) > > loff_t endbyte; > > int err; > > size_t count = 0; > > - bool relock = false; > > int flags = IOMAP_DIOF_PGINVALID_FAIL; > > int ilock_flags = 0; > > > > if (iocb->ki_flags & IOCB_NOWAIT) > > ilock_flags |= BTRFS_ILOCK_TRY; > > + /* > > + * If the write DIO within EOF, use a shared lock > > + */ > > + if (pos + count <= i_size_read(inode)) > > Inode size is now read outside of the inode lock, so it could change > until the lock is taken a few lines below. Good catch. This should be re-checked in btrfs_write_check(). Thanks. > > > + ilock_flags |= BTRFS_ILOCK_SHARED; > > + else if (iocb->ki_flags & IOCB_NOWAIT) > > + return -EAGAIN; > > > > err = btrfs_inode_lock(inode, ilock_flags); > > Is it necessary to revalidate that 'pos + count < i_size' still holds > when the lock was taken as SHARED? Yes, and convert the lock, if not. -- Goldwyn
