REQ_NOAIT cleanups

REQ_NOAIT cleanups

[Christoph Hellwig]

Hi all,

this series cleans up spurious code related to REQ_NOWAIT handling.

I have block layer work depending on this pending, so merging it through
the block tree would be helpful.

Diffstat:
 fs/direct-io.c      |   15 ++++-----------
 include/linux/bio.h |    1 -
 2 files changed, 4 insertions(+), 12 deletions(-)

[PATCH 1/2] direct-io: remove IOCB_NOWAIT support

[Christoph Hellwig]

None of the file systems using the legacy direct I/O code actually sets
FMODE_NOWAIT, and if they did this would not work, as the write locking
could not handle the retry.  Remove this dead code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/direct-io.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 2267f5ae7f77..9e4976716985 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -502,12 +502,8 @@ static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
 	const enum req_op dio_op = dio->opf & REQ_OP_MASK;
 	bool should_dirty = dio_op == REQ_OP_READ && dio->should_dirty;
 
-	if (err) {
-		if (err == BLK_STS_AGAIN && (bio->bi_opf & REQ_NOWAIT))
-			dio->io_error = -EAGAIN;
-		else
-			dio->io_error = -EIO;
-	}
+	if (err)
+		dio->io_error = -EIO;
 
 	if (dio->is_async && should_dirty) {
 		bio_check_pages_dirty(bio);	/* transfers ownership */
@@ -1178,13 +1174,10 @@ ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 		dio->is_async = true;
 
 	dio->inode = inode;
-	if (iov_iter_rw(iter) == WRITE) {
+	if (iov_iter_rw(iter) == WRITE)
 		dio->opf = REQ_OP_WRITE | REQ_SYNC | REQ_IDLE;
-		if (iocb->ki_flags & IOCB_NOWAIT)
-			dio->opf |= REQ_NOWAIT;
-	} else {
+	else
 		dio->opf = REQ_OP_READ;
-	}
 
 	/*
 	 * For AIO O_(D)SYNC writes we need to defer completions to a workqueue
-- 
2.53.0

[PATCH 2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN

[Christoph Hellwig]

Commit abb30460bda2 ("block: mark bio_wouldblock_error() bio with
BIO_QUIET") added this to suppress buffer_head warnings, but neither
when this commit was added nor now any buffer_head using code actually
ever sets REQ_NOWAIT which can lead to BLK_STS_AGAIN.

Remove the special handling for now.  If we ever plan to use REQ_NOWAIT
for buffer_head based I/O we're better off handling BLK_STS_AGAIN in
the completion handler as it actually needs to retry the I/O as well.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/bio.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index 85463981d0f5..7597ae4dc52b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -379,7 +379,6 @@ static inline void bio_io_error(struct bio *bio)
 
 static inline void bio_wouldblock_error(struct bio *bio)
 {
-	bio_set_flag(bio, BIO_QUIET);
 	bio->bi_status = BLK_STS_AGAIN;
 	bio_endio(bio);
 }
-- 
2.53.0

Re: [PATCH 1/2] direct-io: remove IOCB_NOWAIT support

[Damien Le Moal]

On 2026/05/18 8:33, Christoph Hellwig wrote:
> None of the file systems using the legacy direct I/O code actually sets
> FMODE_NOWAIT, and if they did this would not work, as the write locking
> could not handle the retry.  Remove this dead code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN

[Damien Le Moal]

On 2026/05/18 8:33, Christoph Hellwig wrote:
> Commit abb30460bda2 ("block: mark bio_wouldblock_error() bio with
> BIO_QUIET") added this to suppress buffer_head warnings, but neither
> when this commit was added nor now any buffer_head using code actually
> ever sets REQ_NOWAIT which can lead to BLK_STS_AGAIN.
> 
> Remove the special handling for now.  If we ever plan to use REQ_NOWAIT
> for buffer_head based I/O we're better off handling BLK_STS_AGAIN in
> the completion handler as it actually needs to retry the I/O as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] direct-io: remove IOCB_NOWAIT support

[Jan Kara]

On Mon 18-05-26 08:33:29, Christoph Hellwig wrote:
> None of the file systems using the legacy direct I/O code actually sets
> FMODE_NOWAIT, and if they did this would not work, as the write locking
> could not handle the retry.  Remove this dead code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I agree there's no point in bothering with this. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/direct-io.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 2267f5ae7f77..9e4976716985 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -502,12 +502,8 @@ static blk_status_t dio_bio_complete(struct dio *dio, struct bio *bio)
>  	const enum req_op dio_op = dio->opf & REQ_OP_MASK;
>  	bool should_dirty = dio_op == REQ_OP_READ && dio->should_dirty;
>  
> -	if (err) {
> -		if (err == BLK_STS_AGAIN && (bio->bi_opf & REQ_NOWAIT))
> -			dio->io_error = -EAGAIN;
> -		else
> -			dio->io_error = -EIO;
> -	}
> +	if (err)
> +		dio->io_error = -EIO;
>  
>  	if (dio->is_async && should_dirty) {
>  		bio_check_pages_dirty(bio);	/* transfers ownership */
> @@ -1178,13 +1174,10 @@ ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  		dio->is_async = true;
>  
>  	dio->inode = inode;
> -	if (iov_iter_rw(iter) == WRITE) {
> +	if (iov_iter_rw(iter) == WRITE)
>  		dio->opf = REQ_OP_WRITE | REQ_SYNC | REQ_IDLE;
> -		if (iocb->ki_flags & IOCB_NOWAIT)
> -			dio->opf |= REQ_NOWAIT;
> -	} else {
> +	else
>  		dio->opf = REQ_OP_READ;
> -	}
>  
>  	/*
>  	 * For AIO O_(D)SYNC writes we need to defer completions to a workqueue
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] direct-io: remove IOCB_NOWAIT support

[Christian Brauner]

On Mon, May 18, 2026 at 08:33:29AM +0200, Christoph Hellwig wrote:
> None of the file systems using the legacy direct I/O code actually sets
> FMODE_NOWAIT, and if they did this would not work, as the write locking
> could not handle the retry.  Remove this dead code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH 2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN

[Jan Kara]

On Mon 18-05-26 08:33:30, Christoph Hellwig wrote:
> Commit abb30460bda2 ("block: mark bio_wouldblock_error() bio with
> BIO_QUIET") added this to suppress buffer_head warnings, but neither
> when this commit was added nor now any buffer_head using code actually
> ever sets REQ_NOWAIT which can lead to BLK_STS_AGAIN.
> 
> Remove the special handling for now.  If we ever plan to use REQ_NOWAIT
> for buffer_head based I/O we're better off handling BLK_STS_AGAIN in
> the completion handler as it actually needs to retry the I/O as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

So I agree about the bh argument but bio_wouldblock_error() is not specific
to bh path in any way, is it? So do we expect all bi_end_io handlers to
check for BLK_STS_AGAIN and avoid complaining to logs in case of that? Not
that all of them would be currently checking for BIO_QUIET either so I'm
mostly trying to figure out what's the expected handling.

								Honza

> ---
>  include/linux/bio.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 85463981d0f5..7597ae4dc52b 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -379,7 +379,6 @@ static inline void bio_io_error(struct bio *bio)
>  
>  static inline void bio_wouldblock_error(struct bio *bio)
>  {
> -	bio_set_flag(bio, BIO_QUIET);
>  	bio->bi_status = BLK_STS_AGAIN;
>  	bio_endio(bio);
>  }
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN

[Christoph Hellwig]

On Mon, May 18, 2026 at 02:34:49PM +0200, Jan Kara wrote:
> > Commit abb30460bda2 ("block: mark bio_wouldblock_error() bio with
> > BIO_QUIET") added this to suppress buffer_head warnings, but neither
> > when this commit was added nor now any buffer_head using code actually
> > ever sets REQ_NOWAIT which can lead to BLK_STS_AGAIN.
> > 
> > Remove the special handling for now.  If we ever plan to use REQ_NOWAIT
> > for buffer_head based I/O we're better off handling BLK_STS_AGAIN in
> > the completion handler as it actually needs to retry the I/O as well.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> So I agree about the bh argument but bio_wouldblock_error() is not specific
> to bh path in any way, is it? So do we expect all bi_end_io handlers to
> check for BLK_STS_AGAIN and avoid complaining to logs in case of that? Not
> that all of them would be currently checking for BIO_QUIET either so I'm
> mostly trying to figure out what's the expected handling.

File system would need special handling for BLK_STS_AGAIN were we to wire
it up again (in upstream we have the dead direct-io code removed in
patch 1, and an incorrect use in iomap polled I/O I sent a separate patch
for), because simply bubbling it up as -EAGAIN will generally do the
wrong thing.  We ran into file system corruption last time we did it due
to non-idempotency of writes and general locking issues with just
retrying at a higher level.

Thus the rough consensus was that If we want to support fs-issued
REQ_NOWAIT I/O we'd first need to add infrastructure to retry them from
a blocking context at the bio-level, preferably in the block layer
itself.

Re: [PATCH 2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN

[Jan Kara]

On Mon 18-05-26 14:51:09, Christoph Hellwig wrote:
> On Mon, May 18, 2026 at 02:34:49PM +0200, Jan Kara wrote:
> > > Commit abb30460bda2 ("block: mark bio_wouldblock_error() bio with
> > > BIO_QUIET") added this to suppress buffer_head warnings, but neither
> > > when this commit was added nor now any buffer_head using code actually
> > > ever sets REQ_NOWAIT which can lead to BLK_STS_AGAIN.
> > > 
> > > Remove the special handling for now.  If we ever plan to use REQ_NOWAIT
> > > for buffer_head based I/O we're better off handling BLK_STS_AGAIN in
> > > the completion handler as it actually needs to retry the I/O as well.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > So I agree about the bh argument but bio_wouldblock_error() is not specific
> > to bh path in any way, is it? So do we expect all bi_end_io handlers to
> > check for BLK_STS_AGAIN and avoid complaining to logs in case of that? Not
> > that all of them would be currently checking for BIO_QUIET either so I'm
> > mostly trying to figure out what's the expected handling.
> 
> File system would need special handling for BLK_STS_AGAIN were we to wire
> it up again (in upstream we have the dead direct-io code removed in
> patch 1, and an incorrect use in iomap polled I/O I sent a separate patch
> for), because simply bubbling it up as -EAGAIN will generally do the
> wrong thing.  We ran into file system corruption last time we did it due
> to non-idempotency of writes and general locking issues with just
> retrying at a higher level.
> 
> Thus the rough consensus was that If we want to support fs-issued
> REQ_NOWAIT I/O we'd first need to add infrastructure to retry them from
> a blocking context at the bio-level, preferably in the block layer
> itself.

OK, fair. Thanks for explanation. So this flagging with quiet is indeed
pointless. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: REQ_NOAIT cleanups

[Jens Axboe]

On Mon, 18 May 2026 08:33:28 +0200, Christoph Hellwig wrote:
> this series cleans up spurious code related to REQ_NOWAIT handling.
> 
> I have block layer work depending on this pending, so merging it through
> the block tree would be helpful.
> 
> Diffstat:
>  fs/direct-io.c      |   15 ++++-----------
>  include/linux/bio.h |    1 -
>  2 files changed, 4 insertions(+), 12 deletions(-)
> 
> [...]

Applied, thanks!

[1/2] direct-io: remove IOCB_NOWAIT support
      commit: ef9049ec8b9fd6c508832d9f7ab12029f3355102
[2/2] block: don't set BIO_QUIET for BLK_STS_AGAIN
      commit: 481105a949c8d11f7aa770b45fc4c8efcc53f205

Best regards,
-- 
Jens Axboe