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

[Dave Chinner <david@fromorbit.com>]

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.

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...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com