We rely the block layer always being able to send a bio of size
atomic_write_unit_max without being required to split it due to request
queue or other bio limits.

A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
and each vector is at worst case the device logical block size from
direct IO alignment requirement.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-settings.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d151be394c98..57d487a00c64 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -213,6 +213,18 @@ void blk_queue_atomic_write_boundary_bytes(struct request_queue *q,
 }
 EXPORT_SYMBOL(blk_queue_atomic_write_boundary_bytes);
 
+static unsigned int blk_queue_max_guaranteed_bio_size_sectors(
+					struct request_queue *q)
+{
+	struct queue_limits *limits = &q->limits;
+	unsigned int max_segments = min_t(unsigned int, BIO_MAX_VECS,
+					limits->max_segments);
+	/*  Limit according to dev sector size as we only support direct-io */
+	unsigned int limit = max_segments * queue_logical_block_size(q);
+
+	return rounddown_pow_of_two(limit >> SECTOR_SHIFT);
+}
+
 /**
  * blk_queue_atomic_write_unit_min_sectors - smallest unit that can be written
  * atomically to the device.
@@ -223,8 +235,10 @@ void blk_queue_atomic_write_unit_min_sectors(struct request_queue *q,
 					     unsigned int sectors)
 {
 	struct queue_limits *limits = &q->limits;
+	unsigned int guaranteed_sectors =
+		blk_queue_max_guaranteed_bio_size_sectors(q);
 
-	limits->atomic_write_unit_min_sectors = sectors;
+	limits->atomic_write_unit_min_sectors = min(guaranteed_sectors, sectors);
 }
 EXPORT_SYMBOL(blk_queue_atomic_write_unit_min_sectors);
 
@@ -238,8 +252,10 @@ void blk_queue_atomic_write_unit_max_sectors(struct request_queue *q,
 					     unsigned int sectors)
 {
 	struct queue_limits *limits = &q->limits;
+	unsigned int guaranteed_sectors =
+		blk_queue_max_guaranteed_bio_size_sectors(q);
 
-	limits->atomic_write_unit_max_sectors = sectors;
+	limits->atomic_write_unit_max_sectors = min(guaranteed_sectors, sectors);
 }
 EXPORT_SYMBOL(blk_queue_atomic_write_unit_max_sectors);
 
-- 
2.31.1

On Fri, Sep 29, 2023 at 10:27:07AM +0000, John Garry wrote:
> We rely the block layer always being able to send a bio of size
> atomic_write_unit_max without being required to split it due to request
> queue or other bio limits.
> 
> A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
> and each vector is at worst case the device logical block size from
> direct IO alignment requirement.

A bio can have more than BIO_MAX_VECS if you use bio_init.

> +static unsigned int blk_queue_max_guaranteed_bio_size_sectors(
> +					struct request_queue *q)
> +{
> +	struct queue_limits *limits = &q->limits;
> +	unsigned int max_segments = min_t(unsigned int, BIO_MAX_VECS,
> +					limits->max_segments);
> +	/*  Limit according to dev sector size as we only support direct-io */

Who is "we", and how tells the caller to only ever use direct I/O?
And how would a type of userspace I/O even matter for low-level
block code.  What if I wanted to use this for file system metadata?

On 09/11/2023 15:13, Christoph Hellwig wrote:
> On Fri, Sep 29, 2023 at 10:27:07AM +0000, John Garry wrote:
>> We rely the block layer always being able to send a bio of size
>> atomic_write_unit_max without being required to split it due to request
>> queue or other bio limits.
>>
>> A bio may contain min(BIO_MAX_VECS, limits->max_segments) vectors,
>> and each vector is at worst case the device logical block size from
>> direct IO alignment requirement.
> A bio can have more than BIO_MAX_VECS if you use bio_init.

Right, FWIW we are only concerned with codepaths which use BIO_MAX_VECS, 
but I suppose that is not good enough as a guarantee.

> 
>> +static unsigned int blk_queue_max_guaranteed_bio_size_sectors(
>> +					struct request_queue *q)
>> +{
>> +	struct queue_limits *limits = &q->limits;
>> +	unsigned int max_segments = min_t(unsigned int, BIO_MAX_VECS,
>> +					limits->max_segments);
>> +	/*  Limit according to dev sector size as we only support direct-io */
> Who is "we", and how tells the caller to only ever use direct I/O?

I think that this can be dropped as a comment. My earlier series used 
PAGE_SIZE and not sector size here, which I think was proper.

> And how would a type of userspace I/O even matter for low-level
> block code.

It shouldn't do, but we still need to limit according to request queue 
limits.

>  What if I wanted to use this for file system metadata?
> 

As mentioned, I think that the direct-IO comment can be dropped.

Thanks,
John