diff mbox series

[3/7] iomap: Remove lockdep_assert_held()

Message ID 20200522123837.1196-4-rgoldwyn@suse.de
State New, archived
Headers show
Series btrfs direct-io using iomap | expand

Commit Message

Goldwyn Rodrigues May 22, 2020, 12:38 p.m. UTC
From: Goldwyn Rodrigues <rgoldwyn@suse.com>

Filesystems such as btrfs can perform direct I/O without holding the
inode->i_rwsem in some of the cases like writing within i_size.
So, remove the check for lockdep_assert_held() in iomap_dio_rw()

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/iomap/direct-io.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Darrick J. Wong May 28, 2020, 3:40 p.m. UTC | #1
On Fri, May 22, 2020 at 07:38:33AM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> Filesystems such as btrfs can perform direct I/O without holding the
> inode->i_rwsem in some of the cases like writing within i_size.
> So, remove the check for lockdep_assert_held() in iomap_dio_rw()
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/iomap/direct-io.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index f88ba6e7f6af..e4addfc58107 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -416,8 +416,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  	struct blk_plug plug;
>  	struct iomap_dio *dio;
>  
> -	lockdep_assert_held(&inode->i_rwsem);
> -

I could've sworn that I saw a reply from Dave asking to hoist this check
into all the /other/ iomap_dio_rw callers, but I can't find it and maybe
I just dreamed the whole thing.

Also, please cc fsdevel any time you make change to fs/iomap/, even if
we've already reviewed the patches.

--D

>  	if (!count)
>  		return 0;
>  
> -- 
> 2.25.0
>
Goldwyn Rodrigues May 28, 2020, 4:45 p.m. UTC | #2
On  8:40 28/05, Darrick J. Wong wrote:
> On Fri, May 22, 2020 at 07:38:33AM -0500, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > Filesystems such as btrfs can perform direct I/O without holding the
> > inode->i_rwsem in some of the cases like writing within i_size.
> > So, remove the check for lockdep_assert_held() in iomap_dio_rw()
> > 
> > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/iomap/direct-io.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> > index f88ba6e7f6af..e4addfc58107 100644
> > --- a/fs/iomap/direct-io.c
> > +++ b/fs/iomap/direct-io.c
> > @@ -416,8 +416,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> >  	struct blk_plug plug;
> >  	struct iomap_dio *dio;
> >  
> > -	lockdep_assert_held(&inode->i_rwsem);
> > -
> 
> I could've sworn that I saw a reply from Dave asking to hoist this check
> into all the /other/ iomap_dio_rw callers, but I can't find it and maybe
> I just dreamed the whole thing.

It did happen! However, hch mentioned it is not required [1].
I did promise him to remove the entire concept of dio_sem
locking in btrfs, and just rely on inode->i_mutex. It is still a work in
progress.

> 
> Also, please cc fsdevel any time you make change to fs/iomap/, even if
> we've already reviewed the patches.
> 

Yes, missed that. Sorry.

[1] https://www.spinics.net/lists/linux-btrfs/msg96016.html
diff mbox series

Patch

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index f88ba6e7f6af..e4addfc58107 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -416,8 +416,6 @@  iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 	struct blk_plug plug;
 	struct iomap_dio *dio;
 
-	lockdep_assert_held(&inode->i_rwsem);
-
 	if (!count)
 		return 0;