[sheepdog] [PATCH v2 01/17] block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit bytes
Eric Blake
eblake at redhat.com
Wed Apr 29 14:53:32 CEST 2020
On 4/29/20 12:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2020 1:09, Eric Blake wrote:
>> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> The function is called from 64bit io handlers, and bytes is just passed
>>> to throttle_account() which is 64bit too (unsigned though). So, let's
>>> convert intermediate argument to 64bit too.
>>
>> My audit for this patch:
>>
>> Caller has 32-bit, this patch now causes widening which is safe:
>> block/block-backend.c: blk_do_preadv() passes 'unsigned int'
>> block/block-backend.c: blk_do_pwritev_part() passes 'unsigned int'
>> block/throttle.c: throttle_co_pwrite_zeroes() passes 'int'
>> block/throttle.c: throttle_co_pdiscard() passes 'int'
>>
>> Caller has 64-bit, this patch fixes potential bug where pre-patch
>> could narrow, except it's easy enough to trace that callers are still
>> capped at 2G actions:
>> block/throttle.c: throttle_co_preadv() passes 'uint64_t'
>> block/throttle.c: throttle_co_pwritev() passes 'uint64_t'
>>
>> Implementation in question: block/throttle-groups.c
>> throttle_group_co_io_limits_intercept() takes 'unsigned int bytes' and
>> uses it:
>> argument to util/throttle.c throttle_account(uint64_t)
>>
>> All safe: it patches a latent bug, and does not introduce any 64-bit
>> gotchas once throttle_co_p{read,write}v are relaxed, and assuming
>> throttle_account() is not buggy.
>
> Should I add this all to commit message?
It never hurts to provide such rationale ;)
>
>>
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
>>> ---
>>> include/block/throttle-groups.h | 2 +-
>>> block/throttle-groups.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Eric Blake <eblake at redhat.com>
>>
>
> Thanks for careful review!
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
More information about the sheepdog
mailing list