Re: warnings complaining IOMAP_DELALLOC blocks in iomap_dio_actor from generic/446

[Eryu Guan <eguan@redhat.com>]

On Tue, Jul 25, 2017 at 08:02:05AM +1000, Dave Chinner wrote:
> On Tue, Jul 25, 2017 at 01:51:25AM +0800, Eryu Guan wrote:
> > On Mon, Jul 24, 2017 at 09:05:51AM -0700, Darrick J. Wong wrote:
> > > On Mon, Jul 24, 2017 at 10:50:19PM +0800, Eryu Guan wrote:
> > And I'm wondering what's the bigger problem of letting the dio path take
> > MMAPLOCK too to serialize against mmap page faults? e.g.
> > xfs_file_dio_aio_read() takes XFS_MMAPLOCK_SHRED and
> > xfs_filemap_page_mkwrite() takes XFS_MMAPLOCK_EXCL.
> 
> direct IO calls get_user_pages() which can trigger page faults and
> that means we can't hold any lock that is taken in the page fault
> path.

Ah, that's what I missed, thanks!

> 
> It's the same reason we have the MMAPLOCK in the first place - we
> can't use the IOLOCK in the page fault path because copy-in/copy-out
> in the buffered IO path can trigger page faults, hence we need some
> other lock that we can use to serialise page faults against extent
> operations (like fallocate).
> 
> > > It looks like the end result of a dioread/mmapwrite collision is that
> > > the dio reader gets -EIO.  Would it be better to return a short read?
> > 
> > Yes, right now dio read gets EIO in this case. I can't tell which one is
> > better, if the whole dio vs mmap is not recommended, EIO seems to be a
> > strong signal that indicates "don't do this " :)
> 
> $ man 2 open
> .....
> 	.... Likewise, applications should avoid  mixing  mmap(2)
> 	of files with direct I/O to the same files.
> ....
> 
> That said, EIO is extremely unfriendly - a short read would be much
> better as a properly written app will simply try to read the bit it
> didn't get again, whereas EIO tends to be an indication of severe
> failure to the application...

The problem is dio read could hit delalloc blocks on its first read and
return 0, how can we tell between a short read and a real EOF? I may
miss something again ..

Thanks,
Eryu