Re: [Patch v9 05/10] block: Add core atomic write support

[John Garry <john.g.garry@oracle.com>]

On 21/06/2024 07:09, Hannes Reinecke wrote:
> On 6/20/24 14:53, John Garry wrote:
> [ .. ]
>> +/*
>> + * Returns max guaranteed bytes which we can fit in a bio.
>> + *
>> + * We request that an atomic_write is ITER_UBUF iov_iter (so a single 
>> vector),
>> + * so we assume that we can fit in at least PAGE_SIZE in a segment, 
>> apart from
>> + * the first and last segments.
>> + */
>> +static
>> +unsigned int blk_queue_max_guaranteed_bio(struct queue_limits *lim)
>> +{
>> +    unsigned int max_segments = min(BIO_MAX_VECS, lim->max_segments);
>> +    unsigned int length;
>> +
>> +    length = min(max_segments, 2) * lim->logical_block_size;
>> +    if (max_segments > 2)
>> +        length += (max_segments - 2) * PAGE_SIZE;
>> +
>> +    return length;
>> +}
>> +
> Now you got me confused.
> 
> Why is the length of an atomic write two times the logical block size?

It's not just that.

> And even if it does, shouldn't an atomic write be aligned to the logical 
> block size, so why would you need to add two additional PAGE_SIZE worth
> of length?
> And even if _that_ would be okay, why PAGE_SIZE? We're trying really 
> hard to get away from implicit PAGE_SIZE assumptions when doing I/O ...
> 

We need to know what is the maximum size which we can guarantee not to 
need to split. And we work on basis of worst case scenario, i.e. least 
efficient packing of data into iovec[]. However we require a UBUF iter 
and that iter will be aligned to LBS, that would mean that first and 
last segment would be at least logical block size and middle segments 
would be at least PAGE_SIZE.

Thanks,
John