Re: [PATCH v2 1/1] iomap: propagate nowait to block layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/4/25 16:07, Christoph Hellwig wrote:
On Tue, Mar 04, 2025 at 12:18:07PM +0000, Pavel Begunkov wrote:
  	    ((dio->flags & IOMAP_DIO_NEED_SYNC) && !use_fua) ||
-	    ((dio->flags & IOMAP_DIO_WRITE) && pos >= i_size_read(inode)))
+	    ((dio->flags & IOMAP_DIO_WRITE) && pos >= i_size_read(inode))) {
  		dio->flags &= ~IOMAP_DIO_CALLER_COMP;
+ if (!is_sync_kiocb(dio->iocb) &&
+		    (dio->iocb->ki_flags & IOCB_NOWAIT))
+			return -EAGAIN;

Black magic without comments explaining it.

I can copy the comment from below if you wish.

+	if (!is_sync_kiocb(dio->iocb) && (dio->iocb->ki_flags & IOCB_NOWAIT)) {
+		/*
+		 * This is nonblocking IO, and we might need to allocate
+		 * multiple bios. In this case, as we cannot guarantee that
+		 * one of the sub bios will not fail getting issued FOR NOWAIT
+		 * and as error results are coalesced across all of them, ask
+		 * for a retry of this from blocking context.
+		 */
+		if (bio_iov_vecs_to_alloc(dio->submit.iter, BIO_MAX_VECS + 1) >
+					  BIO_MAX_VECS)

This is not very accurate in times of multi-page bvecs and large order
folios all over.

bio_iov_vecs_to_alloc() can overestimate, i.e. the check might return
-EAGAIN in more cases than required but not the other way around,
that should be enough for a fix such as this patch. Or did I maybe
misunderstood you?

I think you really need to byte the bullet and support for early returns
from the non-blocking bio submission path.

Assuming you're suggesting to implement that, I can't say I'm excited by
the idea of reworking a non trivial chunk of block layer to fix a problem
and then porting it up to some 5.x, especially since it was already
attempted before by someone and ultimately got reverted.

--
Pavel Begunkov





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux