Skip to content
Snippets Groups Projects
  • Vladimir Sementsov-Ogievskiy's avatar
    801625e6
    block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit bytes · 801625e6
    Vladimir Sementsov-Ogievskiy authored
    
    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.
    
    This patch is a first in the 64-bit-blocklayer series, so we are
    generally moving to int64_t for both offset and bytes parameters on all
    io paths. Main motivation is realization of 64-bit write_zeroes
    operation for fast zeroing large disk chunks, up to the whole disk.
    
    We chose signed type, to be consistent with off_t (which is signed) and
    with possibility for signed return type (where negative value means
    error).
    
    Patch-correctness audit by Eric Blake:
    
      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.
    
    Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
    Reviewed-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Message-Id: <20201211183934.169161-7-vsementsov@virtuozzo.com>
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    801625e6
    History
    block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit bytes
    Vladimir Sementsov-Ogievskiy authored
    
    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.
    
    This patch is a first in the 64-bit-blocklayer series, so we are
    generally moving to int64_t for both offset and bytes parameters on all
    io paths. Main motivation is realization of 64-bit write_zeroes
    operation for fast zeroing large disk chunks, up to the whole disk.
    
    We chose signed type, to be consistent with off_t (which is signed) and
    with possibility for signed return type (where negative value means
    error).
    
    Patch-correctness audit by Eric Blake:
    
      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.
    
    Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
    Reviewed-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Message-Id: <20201211183934.169161-7-vsementsov@virtuozzo.com>
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>