Re: [PATCH 3/3] iomap: use filemap_range_needs_writeback() for O_DIRECT reads

On 2/9/21 12:51 AM, Christoph Hellwig wrote:

On Mon, Feb 08, 2021 at 07:30:08PM -0700, Jens Axboe wrote:

quoted

+		if (iocb->ki_flags & IOCB_NOWAIT) {
+			if (filemap_range_needs_writeback(mapping, pos, end)) {
+				ret = -EAGAIN;
+				goto out_free_dio;
+			}
+			flags |= IOMAP_NOWAIT;
+		}
 		if (iter_is_iovec(iter))
 			dio->flags |= IOMAP_DIO_DIRTY;
 	} else {
+		if (iocb->ki_flags & IOCB_NOWAIT) {
+			if (filemap_range_has_page(mapping, pos, end)) {
+				ret = -EAGAIN;
+				goto out_free_dio;
+			}
+			flags |= IOMAP_NOWAIT;
+		}
+
 		flags |= IOMAP_WRITE;
 		dio->flags |= IOMAP_DIO_WRITE;
 

@@ -478,14 +493,6 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 			dio->flags |= IOMAP_DIO_WRITE_FUA;
 	}
 
-	if (iocb->ki_flags & IOCB_NOWAIT) {
-		if (filemap_range_has_page(mapping, pos, end)) {
-			ret = -EAGAIN;
-			goto out_free_dio;
-		}
-		flags |= IOMAP_NOWAIT;
-	}

looking at this I really hate the scheme with the potential racyness
and duplicated page looksups.

Me too

Why can't we pass a nonblock flag to filemap_write_and_wait_range
and invalidate_inode_pages2_range that makes them return -EAGAIN
when they would block to clean this whole mess up?

We could, but that's a _lot_ of surgery. I'd rather live with the
slight race for now instead of teaching writepages, page laundering,
etc about IOCB_NOWAIT.

I do think that's a worthy long term goal, but we dio read situation
is bad enough that it warrants a quicker fix.

-- 
Jens Axboe