Re: [PATCH 0/3] Transient errors in Direct I/O

Re: [PATCH 0/3] Transient errors in Direct I/O

[Dave Chinner]

[ Please cc the XFS list on XFS and iomap infrastructure changes.]

On Fri, Jun 05, 2020 at 03:48:35PM -0500, Goldwyn Rodrigues wrote:
> In current scenarios, for XFS, it would mean that a page invalidation
> would end up being a writeback error. So, if iomap returns zero, fall
> back to biffered I/O. XFS has never supported fallback to buffered I/O.
> I hope it is not "never will" ;)

I wouldn't say "never", but we are not going to change XFS behaviour
because btrfs has a page invalidation vs DIO bug in it...

If you want to make a specific filesystem fall back to buffered IO
in this case, pass a new flag into iomap_dio_rw() to conditionally
abort the DIO on invalidation failure and let filesystem
implementations opt-in to use that flag.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/3] Transient errors in Direct I/O

[Dave Chinner]

On Wed, Jun 10, 2020 at 12:59:00PM +1000, Dave Chinner wrote:
> [ Please cc the XFS list on XFS and iomap infrastructure changes.]
> 
> On Fri, Jun 05, 2020 at 03:48:35PM -0500, Goldwyn Rodrigues wrote:
> > In current scenarios, for XFS, it would mean that a page invalidation
> > would end up being a writeback error. So, if iomap returns zero, fall
> > back to biffered I/O. XFS has never supported fallback to buffered I/O.
> > I hope it is not "never will" ;)
> 
> I wouldn't say "never", but we are not going to change XFS behaviour
> because btrfs has a page invalidation vs DIO bug in it...

Let me point out a specific "oh shit, I didn't think of that" sort
of problem that your blind fallback to buffered IO causes. Do this
via direct IO:

	pwritev2(RWF_NOWAIT)

now have it fail invalidation in the direct IO path and fallback to
buffered write. What does buffered write do with it?


	if (iocb->ki_flags & IOCB_NOWAIT)
		return -EOPNOTSUPP;

Yup, userspace gets a completely spurious and bogus -EOPNOTSUPP
error to pwritev2() because some 3rd party is accessing the same
file via mmap or buffered IO.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/3] Transient errors in Direct I/O

[Goldwyn Rodrigues]

On 15:05 10/06, Dave Chinner wrote:
> On Wed, Jun 10, 2020 at 12:59:00PM +1000, Dave Chinner wrote:
> > [ Please cc the XFS list on XFS and iomap infrastructure changes.]
> > 
> > On Fri, Jun 05, 2020 at 03:48:35PM -0500, Goldwyn Rodrigues wrote:
> > > In current scenarios, for XFS, it would mean that a page invalidation
> > > would end up being a writeback error. So, if iomap returns zero, fall
> > > back to biffered I/O. XFS has never supported fallback to buffered I/O.
> > > I hope it is not "never will" ;)
> > 
> > I wouldn't say "never", but we are not going to change XFS behaviour
> > because btrfs has a page invalidation vs DIO bug in it...
> 
> Let me point out a specific "oh shit, I didn't think of that" sort
> of problem that your blind fallback to buffered IO causes. Do this
> via direct IO:
> 
> 	pwritev2(RWF_NOWAIT)
> 
> now have it fail invalidation in the direct IO path and fallback to
> buffered write. What does buffered write do with it?
> 
> 
> 	if (iocb->ki_flags & IOCB_NOWAIT)
> 		return -EOPNOTSUPP;
> 
> Yup, userspace gets a completely spurious and bogus -EOPNOTSUPP
> error to pwritev2() because some 3rd party is accessing the same
> file via mmap or buffered IO.
> 

Oh shit, I didn't think about that!

I think adding a flag to iomap_dio_rw() to return in case of page
invalidation failure is the best option for now.

-- 
Goldwyn