Re: [PATCH 5/5] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA

[Christoph Hellwig <hch@lst.de>]

On Sat, Jul 01, 2017 at 12:03:06AM -0700, Darrick J. Wong wrote:
> On Fri, Jun 30, 2017 at 07:37:40PM +0200, Christoph Hellwig wrote:
> > On Fri, Jun 30, 2017 at 01:51:10PM +0200, Andreas Gruenbacher wrote:
> > > Also, ext4 no longer calls inode_lock or inode_lock_shared; that needs
> > > to be added back for consistency between reading i_size and walking
> > > the file extents.
> > 
> > At least for XFS we never had such a consistency as we never took
> > the iolock (aka i_rwsem).
> 
> Do we need it?

For XFS I'm pretty sure we don't.  The lseek is fundamentally safe
without it due to the ilock.

> The non-ext4 mechanical parts look ok to me (and test out ok), but I
> want to be sure that we don't create a locking mess.  Dave complained in
> an earlier thread about lockdep problems because the old code took the
> ilock and then started locking pages; since we take the ILOCK
> during ->iomap_begin, drop it, and only take page locks during
> page_cache_seek_hole_data (which is called from the iomap actor) I think
> that particular problem goes away.

The old code took the ilook in the seek hole/data helper, which had
two problems:  it double locked the ilock as we take it in iomap,
and it hols the ilock over the page cache calls.  None of which happen
with this code.