Re: [PATCH 11/15] btrfs: Use inode_lock_shared() for direct writes within EOF

[Goldwyn Rodrigues <rgoldwyn@suse.de>]

On 10:52 22/09, Josef Bacik wrote:
> On 9/21/20 10:43 AM, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > Direct writes within EOF are safe to be performed with inode shared lock
> > to improve parallelization with other direct writes or reads because EOF
> > is not changed and there is no race with truncate().
> > 
> > Direct reads are already performed under shared inode lock.
> > 
> > This patch is precursor to removing btrfs_inode->dio_sem.
> > 
> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > ---
> >   fs/btrfs/file.c | 33 +++++++++++++++++++++------------
> >   1 file changed, 21 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index d9c3be19d7b3..50092d24eee2 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1977,7 +1977,6 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> >   	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> >   	loff_t pos;
> >   	ssize_t written = 0;
> > -	bool relock = false;
> >   	ssize_t written_buffered;
> >   	loff_t endbyte;
> >   	int err;
> > @@ -1986,6 +1985,15 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
> >   	if (iocb->ki_flags & IOCB_NOWAIT)
> >   		ilock_flags |= BTRFS_ILOCK_TRY;
> > +	/*
> > +	 * If the write DIO within EOF,  use a shared lock
> > +	 */
> > +	if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
> > +		ilock_flags |= BTRFS_ILOCK_SHARED;
> > +	else if (iocb->ki_flags & IOCB_NOWAIT)
> > +		return -EAGAIN;
> > +
> > +relock:
> 
> Huh?  Why are you making it so EOF extending NOWAIT writes now fail?  We are
> still using ILOCK_TRY here, so we may still not block, am I missing
> something? Thanks,
> 

Yes, this is incorrect. I had thought of this would block on disk space
allocations. But did not consider the prealloc case.

I am removing this check to match the previous behavior.

-- 
Goldwyn