Re: [RFC PATCH 0/8] block: fix bio_add_XXX_page() return type

[Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>]

Christoph,

On 5/24/21 00:35, Christoph Hellwig wrote:
> On Fri, May 21, 2021 at 12:30:45PM +0100, Matthew Wilcox wrote:
>> On Wed, May 19, 2021 at 11:22:47PM -0700, Chaitanya Kulkarni wrote:
>>> The helper functions bio_add_XXX_page() returns the length which is
>>> unsigned int but the return type of those functions is defined
>>> as int instead of unsigned int.
>> I've been thinking about this for a few weeks as part of the folio
>> patches:
>>
>> https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/
>>
>>  - len and off are measured in bytes
>>  - neither are permitted to be negative
>>  - for efficiency we only permit them to be up to 4GB
>>
>> I therefore believe the correct type for these parameters to be size_t,
>> and we should range-check them if they're too large.  they should
>> actually always fit within the page that they're associated with, but
>> people do allocate non-compound pages and i'm not trying to break that
>> today.
>>
>> using size_t makes it clear that these are byte counts, not (eg) sector
>> counts.  i do think it's good to make the return value unsigned so we
>> don't have people expecting a negative errno on failure.
> I think the right type is bool.  We always return either 0 or the full
> length we tried to add.  Instead of optimizing for a partial add (which
> only makes sense for bio_add_hw_page anyway), I'd rather make the
> interface as simple as possible.
>

Is above comment is on this series or on the API present in the folio
patches [1] ?

Since if we change the return type to bool for the functions in
question [2] in this series we also need to modify the callers, I'm not sure
that is worth it though.

Please confirm.

[1]
https://lore.kernel.org/linux-fsdevel/20210505150628.111735-72-willy@infradead.org/


[2]
bio_add_hw_page()
bio_add_pc_page()
bio_add_zone_append_page()
bio_add page()