public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCHSET 0/3] Improve FMODE_NOWAIT coverage
@ 2023-05-09 15:19 Jens Axboe
  2023-05-09 15:19 ` [PATCH 1/3] net: set FMODE_NOWAIT for sockets Jens Axboe
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Jens Axboe @ 2023-05-09 15:19 UTC (permalink / raw)
  To: io-uring; +Cc: torvalds

Hi,

io_uring jumps through some hoops to check block devices and sockets
for sanity wrt nonblocking read/write attempts. We can get rid of that
if we just flag sockets and block devices as being sane in that regard.
Patches 1-2 do that for sockets and block devices, and then patch 3 can
remove some cruft on the io_uring side that is pretty ugly.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] net: set FMODE_NOWAIT for sockets
  2023-05-09 15:19 [PATCHSET 0/3] Improve FMODE_NOWAIT coverage Jens Axboe
@ 2023-05-09 15:19 ` Jens Axboe
  2023-05-11  8:03   ` Paolo Abeni
  2023-05-09 15:19 ` [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it Jens Axboe
  2023-05-09 15:19 ` [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT Jens Axboe
  2 siblings, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2023-05-09 15:19 UTC (permalink / raw)
  To: io-uring
  Cc: torvalds, Jens Axboe, David S. Miller, Eric Dumazet,
	Jakub Kicinski, netdev

The socket read/write functions deal with O_NONBLOCK and IOCB_NOWAIT
just fine, so we can flag them as being FMODE_NOWAIT compliant. With
this, we can remove socket special casing in io_uring when checking
if a file type is sane for nonblocking IO, and it's also the defined
way to flag file types as such in the kernel.

Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: [email protected]
Signed-off-by: Jens Axboe <[email protected]>
---
 net/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index a7b4b37d86df..6861dbbfadb6 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -471,6 +471,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
 		return file;
 	}
 
+	file->f_mode |= FMODE_NOWAIT;
 	sock->file = file;
 	file->private_data = sock;
 	stream_open(SOCK_INODE(sock), file);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-05-09 15:19 [PATCHSET 0/3] Improve FMODE_NOWAIT coverage Jens Axboe
  2023-05-09 15:19 ` [PATCH 1/3] net: set FMODE_NOWAIT for sockets Jens Axboe
@ 2023-05-09 15:19 ` Jens Axboe
  2023-05-10 13:30   ` Christoph Hellwig
  2023-05-09 15:19 ` [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT Jens Axboe
  2 siblings, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2023-05-09 15:19 UTC (permalink / raw)
  To: io-uring; +Cc: torvalds, Jens Axboe, linux-block

We set this unconditionally, but it really should be dependent on if
the underlying device is nowait compliant.

Cc: [email protected]
Signed-off-by: Jens Axboe <[email protected]>
---
 block/fops.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/fops.c b/block/fops.c
index d2e6be4e3d1c..ab750e8a040f 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -481,7 +481,7 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	 * during an unstable branch.
 	 */
 	filp->f_flags |= O_LARGEFILE;
-	filp->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC;
+	filp->f_mode |= FMODE_BUF_RASYNC;
 
 	if (filp->f_flags & O_NDELAY)
 		filp->f_mode |= FMODE_NDELAY;
@@ -494,6 +494,9 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 
+	if (bdev_nowait(bdev))
+		filp->f_mode |= FMODE_NOWAIT;
+
 	filp->private_data = bdev;
 	filp->f_mapping = bdev->bd_inode->i_mapping;
 	filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT
  2023-05-09 15:19 [PATCHSET 0/3] Improve FMODE_NOWAIT coverage Jens Axboe
  2023-05-09 15:19 ` [PATCH 1/3] net: set FMODE_NOWAIT for sockets Jens Axboe
  2023-05-09 15:19 ` [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it Jens Axboe
@ 2023-05-09 15:19 ` Jens Axboe
  2023-05-09 16:48   ` Linus Torvalds
  2 siblings, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2023-05-09 15:19 UTC (permalink / raw)
  To: io-uring; +Cc: torvalds, Jens Axboe

Now that we have both sockets and block devices setting FMODE_NOWAIT
appropriately, we can get rid of all the odd special casing in
__io_file_supports_nowait() and rely soley on FMODE_NOWAIT and
O_NONBLOCK rather than special case sockets and (in particular) bdevs.

Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/io_uring.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3bca7a79efda..7c426584e35a 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1758,11 +1758,6 @@ static void io_iopoll_req_issued(struct io_kiocb *req, unsigned int issue_flags)
 	}
 }
 
-static bool io_bdev_nowait(struct block_device *bdev)
-{
-	return !bdev || bdev_nowait(bdev);
-}
-
 /*
  * If we tracked the file through the SCM inflight mechanism, we could support
  * any file. For now, just ensure that anything potentially problematic is done
@@ -1770,22 +1765,6 @@ static bool io_bdev_nowait(struct block_device *bdev)
  */
 static bool __io_file_supports_nowait(struct file *file, umode_t mode)
 {
-	if (S_ISBLK(mode)) {
-		if (IS_ENABLED(CONFIG_BLOCK) &&
-		    io_bdev_nowait(I_BDEV(file->f_mapping->host)))
-			return true;
-		return false;
-	}
-	if (S_ISSOCK(mode))
-		return true;
-	if (S_ISREG(mode)) {
-		if (IS_ENABLED(CONFIG_BLOCK) &&
-		    io_bdev_nowait(file->f_inode->i_sb->s_bdev) &&
-		    !io_is_uring_fops(file))
-			return true;
-		return false;
-	}
-
 	/* any ->read/write should understand O_NONBLOCK */
 	if (file->f_flags & O_NONBLOCK)
 		return true;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT
  2023-05-09 15:19 ` [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT Jens Axboe
@ 2023-05-09 16:48   ` Linus Torvalds
  0 siblings, 0 replies; 14+ messages in thread
From: Linus Torvalds @ 2023-05-09 16:48 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring

On Tue, May 9, 2023 at 8:19 AM Jens Axboe <[email protected]> wrote:
>
> Now that we have both sockets and block devices setting FMODE_NOWAIT
> appropriately, we can get rid of all the odd special casing in
> __io_file_supports_nowait() and rely soley on FMODE_NOWAIT and
> O_NONBLOCK rather than special case sockets and (in particular) bdevs.

Yup, looks good to me. Thanks.

               Linus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-05-09 15:19 ` [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it Jens Axboe
@ 2023-05-10 13:30   ` Christoph Hellwig
  2023-05-10 15:32     ` Jens Axboe
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2023-05-10 13:30 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, torvalds, linux-block

On Tue, May 09, 2023 at 09:19:09AM -0600, Jens Axboe wrote:
> We set this unconditionally, but it really should be dependent on if
> the underlying device is nowait compliant.

Somehow I only see patch 2 of 3 of whatever series this is supposed to
be in my linux-block mbox, something is broken with your patch sending
script.

The change itself looks fine even standalone, though:

Reviewed-by: Christoph Hellwig <[email protected]>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-05-10 13:30   ` Christoph Hellwig
@ 2023-05-10 15:32     ` Jens Axboe
  2023-06-20  6:18       ` Christoph Hellwig
  0 siblings, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2023-05-10 15:32 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: io-uring, torvalds, linux-block

On 5/10/23 7:30 AM, Christoph Hellwig wrote:
> On Tue, May 09, 2023 at 09:19:09AM -0600, Jens Axboe wrote:
>> We set this unconditionally, but it really should be dependent on if
>> the underlying device is nowait compliant.
> 
> Somehow I only see patch 2 of 3 of whatever series this is supposed to
> be in my linux-block mbox, something is broken with your patch sending
> script.
> 
> The change itself looks fine even standalone, though:
> 
> Reviewed-by: Christoph Hellwig <[email protected]>

Thanks - the 2/3 only is on purpose, 1/3 is a networking ditto and
3/3 is just io_uring now being able to delete some code. So this one
was supposed to be able to stand on its own, should've had the cover
letter for everyone obviously though.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] net: set FMODE_NOWAIT for sockets
  2023-05-09 15:19 ` [PATCH 1/3] net: set FMODE_NOWAIT for sockets Jens Axboe
@ 2023-05-11  8:03   ` Paolo Abeni
  2023-05-11 13:30     ` Jens Axboe
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Abeni @ 2023-05-11  8:03 UTC (permalink / raw)
  To: Jens Axboe, io-uring
  Cc: torvalds, David S. Miller, Eric Dumazet, Jakub Kicinski, netdev

On Tue, 2023-05-09 at 09:19 -0600, Jens Axboe wrote:
> The socket read/write functions deal with O_NONBLOCK and IOCB_NOWAIT
> just fine, so we can flag them as being FMODE_NOWAIT compliant. With
> this, we can remove socket special casing in io_uring when checking
> if a file type is sane for nonblocking IO, and it's also the defined
> way to flag file types as such in the kernel.
> 
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: [email protected]
> Signed-off-by: Jens Axboe <[email protected]>
> ---
>  net/socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/socket.c b/net/socket.c
> index a7b4b37d86df..6861dbbfadb6 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -471,6 +471,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
>  		return file;
>  	}
>  
> +	file->f_mode |= FMODE_NOWAIT;
>  	sock->file = file;
>  	file->private_data = sock;
>  	stream_open(SOCK_INODE(sock), file);

The patch looks sane to me:

Reviewed-by: Paolo Abeni <[email protected]>

I understand the intention is merging patch via the io_uring tree? If
so, no objections on my side: hopefully it should not cause any
conflicts with the netdev tree.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] net: set FMODE_NOWAIT for sockets
  2023-05-11  8:03   ` Paolo Abeni
@ 2023-05-11 13:30     ` Jens Axboe
  0 siblings, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2023-05-11 13:30 UTC (permalink / raw)
  To: Paolo Abeni, io-uring
  Cc: torvalds, David S. Miller, Eric Dumazet, Jakub Kicinski, netdev

On 5/11/23 2:03?AM, Paolo Abeni wrote:
> On Tue, 2023-05-09 at 09:19 -0600, Jens Axboe wrote:
>> The socket read/write functions deal with O_NONBLOCK and IOCB_NOWAIT
>> just fine, so we can flag them as being FMODE_NOWAIT compliant. With
>> this, we can remove socket special casing in io_uring when checking
>> if a file type is sane for nonblocking IO, and it's also the defined
>> way to flag file types as such in the kernel.
>>
>> Cc: "David S. Miller" <[email protected]>
>> Cc: Eric Dumazet <[email protected]>
>> Cc: Jakub Kicinski <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Jens Axboe <[email protected]>
>> ---
>>  net/socket.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/socket.c b/net/socket.c
>> index a7b4b37d86df..6861dbbfadb6 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -471,6 +471,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
>>  		return file;
>>  	}
>>  
>> +	file->f_mode |= FMODE_NOWAIT;
>>  	sock->file = file;
>>  	file->private_data = sock;
>>  	stream_open(SOCK_INODE(sock), file);
> 
> The patch looks sane to me:
> 
> Reviewed-by: Paolo Abeni <[email protected]>
> 
> I understand the intention is merging patch via the io_uring tree? If
> so, no objections on my side: hopefully it should not cause any
> conflicts with the netdev tree.

If it's fine with you guys, then yeah that would make my life easier.
Risk of conflicts should be very low, and trivial if it does occur.
Thanks for the review!

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-05-10 15:32     ` Jens Axboe
@ 2023-06-20  6:18       ` Christoph Hellwig
  2023-06-20  8:22         ` Christoph Hellwig
  2023-06-20 13:24         ` Jens Axboe
  0 siblings, 2 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-06-20  6:18 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, io-uring, linux-block

So it turns out this gets into the way of my planned cleanup to move
all the tatic FMODE_ flags out of this basically full field into a new
static one in file_operations.  Do you think it is ok to go back to
always claiming FMODE_NOWAIT for block devices and then just punt for
the drivers that don't support it like the patch below?

---
From 05a591ac066d9d2d57c4967bbd49c8bc63b04abf Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Tue, 20 Jun 2023 07:53:13 +0200
Subject: block: always set FMODE_NOWAIT

Block devices are the only file_operation that do not set FMODE_NOWAIT
unconditionall in ->open and thus get in the way of a planned cleanup to
move this flags into a static field in file_operations.   Switch to
always set FMODE_NOWAIT and just return -EAGAIN if it isn't actually
supported.  This just affects minor ->submit_bio based drivers as all
blk-mq drivers and the important remappers (dm, md, nvme-multipath)
support nowait I/O.

Signed-off-by: Christoph Hellwig <[email protected]>
---
 block/fops.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 555b1b9ecd2cb9..8068d0c85ae75b 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -505,7 +505,7 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	 * during an unstable branch.
 	 */
 	filp->f_flags |= O_LARGEFILE;
-	filp->f_mode |= FMODE_BUF_RASYNC;
+	filp->f_mode |= FMODE_BUF_RASYNC | FMODE_NOWAIT;
 
 	/*
 	 * Use the file private data to store the holder for exclusive openes.
@@ -519,9 +519,6 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 
-	if (bdev_nowait(bdev))
-		filp->f_mode |= FMODE_NOWAIT;
-
 	filp->f_mapping = bdev->bd_inode->i_mapping;
 	filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
 	return 0;
@@ -563,6 +560,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT)
 		return -EOPNOTSUPP;
 
+	if (!bdev_nowait(bdev))
+		return -EAGAIN;
+
 	size -= iocb->ki_pos;
 	if (iov_iter_count(from) > size) {
 		shorted = iov_iter_count(from) - size;
@@ -585,6 +585,9 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	ssize_t ret = 0;
 	size_t count;
 
+	if (!bdev_nowait(bdev))
+		return -EAGAIN;
+
 	if (unlikely(pos + iov_iter_count(to) > size)) {
 		if (pos >= size)
 			return 0;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-06-20  6:18       ` Christoph Hellwig
@ 2023-06-20  8:22         ` Christoph Hellwig
  2023-06-20 13:24         ` Jens Axboe
  1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2023-06-20  8:22 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, io-uring, linux-block

On Mon, Jun 19, 2023 at 11:18:55PM -0700, Christoph Hellwig wrote:
> So it turns out this gets into the way of my planned cleanup to move
> all the tatic FMODE_ flags out of this basically full field into a new
> static one in file_operations.  Do you think it is ok to go back to
> always claiming FMODE_NOWAIT for block devices and then just punt for
> the drivers that don't support it like the patch below?

Except that the version I posted if of course completly broken as my
testing rig told me.  This is the version that actually works:

---
From 7916df7434e1570978b9c81c65aa1ec1f3396b13 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Tue, 20 Jun 2023 07:53:13 +0200
Subject: block: always set FMODE_NOWAIT

Block devices are the only file_operations that do not set FMODE_NOWAIT
unconditionall in ->open and thus get in the way of a planned cleanup to
move this flags into a static field in file_operations.   Switch to
always set FMODE_NOWAIT and just return -EAGAIN if it isn't actually
supported.  This just affects minor ->submit_bio based drivers as all
blk-mq drivers and the important remappers (dm, md, nvme-multipath)
support nowait I/O.

Signed-off-by: Christoph Hellwig <[email protected]>
---
 block/fops.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 555b1b9ecd2cb9..58c2f65ae4a57e 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -505,7 +505,7 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	 * during an unstable branch.
 	 */
 	filp->f_flags |= O_LARGEFILE;
-	filp->f_mode |= FMODE_BUF_RASYNC;
+	filp->f_mode |= FMODE_BUF_RASYNC | FMODE_NOWAIT;
 
 	/*
 	 * Use the file private data to store the holder for exclusive openes.
@@ -519,9 +519,6 @@ static int blkdev_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 
-	if (bdev_nowait(bdev))
-		filp->f_mode |= FMODE_NOWAIT;
-
 	filp->f_mapping = bdev->bd_inode->i_mapping;
 	filp->f_wb_err = filemap_sample_wb_err(filp->f_mapping);
 	return 0;
@@ -563,6 +560,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT)
 		return -EOPNOTSUPP;
 
+	if ((iocb->ki_flags & IOCB_NOWAIT) && !bdev_nowait(bdev))
+		return -EAGAIN;
+
 	size -= iocb->ki_pos;
 	if (iov_iter_count(from) > size) {
 		shorted = iov_iter_count(from) - size;
@@ -585,6 +585,9 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	ssize_t ret = 0;
 	size_t count;
 
+	if ((iocb->ki_flags & IOCB_NOWAIT) && !bdev_nowait(bdev))
+		return -EAGAIN;
+
 	if (unlikely(pos + iov_iter_count(to) > size)) {
 		if (pos >= size)
 			return 0;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-06-20  6:18       ` Christoph Hellwig
  2023-06-20  8:22         ` Christoph Hellwig
@ 2023-06-20 13:24         ` Jens Axboe
  2023-06-20 13:29           ` Christoph Hellwig
  1 sibling, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2023-06-20 13:24 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: io-uring, linux-block

On 6/20/23 12:18?AM, Christoph Hellwig wrote:
> So it turns out this gets into the way of my planned cleanup to move
> all the tatic FMODE_ flags out of this basically full field into a new
> static one in file_operations.  Do you think it is ok to go back to
> always claiming FMODE_NOWAIT for block devices and then just punt for
> the drivers that don't support it like the patch below?

I think we need stronger justification than that, it's much nicer to
have it in the open path than doing the same check over and over for
each IO.

With your new proposed scheme, why can't the check and FMODE_NOWAIT set
still be in open?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-06-20 13:24         ` Jens Axboe
@ 2023-06-20 13:29           ` Christoph Hellwig
  2023-06-20 14:56             ` Jens Axboe
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2023-06-20 13:29 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Christoph Hellwig, io-uring, linux-block

On Tue, Jun 20, 2023 at 07:24:56AM -0600, Jens Axboe wrote:
> I think we need stronger justification than that, it's much nicer to
> have it in the open path than doing the same check over and over for
> each IO.
> 
> With your new proposed scheme, why can't the check and FMODE_NOWAIT set
> still be in open?

Because I want to move the by now huge number of static flags out of
file->f_mode and into file->f_op.flags.  It's just that with this patch
this one flag isn't static anymore for block devices.  We could also
do two sets of file operations assuming we never allow run-time changes
for QUEUE_FLAG_NOWAIT.  If we care about optimizing fo async I/O on the
few drivers not supporting QUEUE_FLAG_NOWAIT that's probably the next
best thing.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it
  2023-06-20 13:29           ` Christoph Hellwig
@ 2023-06-20 14:56             ` Jens Axboe
  0 siblings, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2023-06-20 14:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: io-uring, linux-block

On 6/20/23 7:29?AM, Christoph Hellwig wrote:
> On Tue, Jun 20, 2023 at 07:24:56AM -0600, Jens Axboe wrote:
>> I think we need stronger justification than that, it's much nicer to
>> have it in the open path than doing the same check over and over for
>> each IO.
>>
>> With your new proposed scheme, why can't the check and FMODE_NOWAIT set
>> still be in open?
> 
> Because I want to move the by now huge number of static flags out of
> file->f_mode and into file->f_op.flags.  It's just that with this patch
> this one flag isn't static anymore for block devices.  We could also
> do two sets of file operations assuming we never allow run-time changes
> for QUEUE_FLAG_NOWAIT.  If we care about optimizing fo async I/O on the
> few drivers not supporting QUEUE_FLAG_NOWAIT that's probably the next
> best thing.

Doing two sets of fops makes sense to me, and then hopefully down the
line we can shave it down to 1 again once everything sets FMODE_NOWAIT.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-06-20 14:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-09 15:19 [PATCHSET 0/3] Improve FMODE_NOWAIT coverage Jens Axboe
2023-05-09 15:19 ` [PATCH 1/3] net: set FMODE_NOWAIT for sockets Jens Axboe
2023-05-11  8:03   ` Paolo Abeni
2023-05-11 13:30     ` Jens Axboe
2023-05-09 15:19 ` [PATCH 2/3] block: mark bdev files as FMODE_NOWAIT if underlying device supports it Jens Axboe
2023-05-10 13:30   ` Christoph Hellwig
2023-05-10 15:32     ` Jens Axboe
2023-06-20  6:18       ` Christoph Hellwig
2023-06-20  8:22         ` Christoph Hellwig
2023-06-20 13:24         ` Jens Axboe
2023-06-20 13:29           ` Christoph Hellwig
2023-06-20 14:56             ` Jens Axboe
2023-05-09 15:19 ` [PATCH 3/3] io_uring: rely solely on FMODE_NOWAIT Jens Axboe
2023-05-09 16:48   ` Linus Torvalds

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox