Re: [PATCH 3/8] iomap: Move lockdep_assert_held() to iomap_dio_rw() calls

Darrick J. Wong
2019-12-21T18:05:49Z

On Sat, Dec 21, 2019 at 05:41:18AM -0800, Christoph Hellwig wrote:
> On Fri, Dec 13, 2019 at 01:57:45PM -0600, 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()
>
> As said last time: in the callers the assert is completely pointless,
> as it is always very close to taking the lock. This was just intended
> to deal with callers not adhering to the iomap_dio_rw calling
> conventins, and moving the assert to the calllers doesn't help with
> that at all.
>
> So if you think you need to remove it do just that and write a changelog
> explaining the why much better.

Uh... that's exactly what this patch does, and the commit message says
that btrfs doesn't need to hold i_rwsem to guarantee concurrency
correctness.

Hm, but maybe the commit message is simply too subtle here? How about:

"Filesystems do not necessarily need i_rwsem to ensure correct operation
with multiple threads, so remove the i_rwsem assertion in iomap_dio_rw.
For example, btrfs can perform directio within i_size without needing to
hold i_rwsem."

--D