Re: [PATCH v6 6/7] xfs: Validate atomic writes

From: John Garry
Date: Tue Oct 01 2024 - 11:49:33 EST


On 01/10/2024 15:48, Darrick J. Wong wrote:
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -679,7 +679,12 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter
*iter,
if (ret != -EAGAIN) {
trace_iomap_dio_invalidate_fail(inode, iomi.pos,
iomi.len);
- ret = -ENOTBLK;
+ if (iocb->ki_flags & IOCB_ATOMIC) {
+ if (ret == -ENOTBLK)
+ ret = -EAGAIN;
I don't follow the logic here -- all the error codes except for EAGAIN
are squashed into ENOTBLK, so why would we let them through for an
atomic write?

Right, the errors apart from EAGAIN are normally squashed to ENOTBLK; but I thought that since we would not do this for IOCB_ATOMIC, then just return the actual error from kiocb_invalidate_pages() - apart from ENOTBLK, which has this special treatment.

But I can always just return EAGAIN for IOCB_ATOMIC when kiocb_invalidate_pages() errors, as you suggest below.


if (ret != -EAGAIN) {
trace_iomap_dio_invalidate_fail(inode, iomi.pos,
iomi.len);

if (iocb->ki_flags & IOCB_ATOMIC) {
/*
* folio invalidation failed, maybe this is
* transient, unlock and see if the caller
* tries again
*/
return -EAGAIN;
} else {
/* fall back to buffered write */
return -ENOTBLK;
}
}

Cheers,
John