Commit cf081fca authored by Eric Blake's avatar Eric Blake Committed by Kevin Wolf
Browse files

block: Track write zero limits in bytes



Another step towards removing sector-based interfaces: convert
the maximum write and minimum alignment values from sectors to
bytes.  Rename the variables to let the compiler check that all
users are converted to the new semantics.

The maximum remains an int as long as BDRV_REQUEST_MAX_SECTORS
is constrained by INT_MAX (this means that we can't even
support a 2G write_zeroes, but just under it) - changing
operation lengths to unsigned or to 64-bits is a much bigger
audit, and debatable if we even want to do it (since at the
core, a 32-bit platform will still have ssize_t as its
underlying limit on write()).

Meanwhile, alignment is changed to 'uint32_t', since it makes no
sense to have an alignment larger than the maximum write, and
less painful to use an unsigned type with well-defined behavior
in bit operations than to have to worry about what happens if
a driver mistakenly supplies a negative alignment.

Add an assert that no one was trying to use sectors to get a
write zeroes larger than 2G, and therefore that a later conversion
to bytes won't be impacted by keeping the limit at 32 bits.

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
parent 8b184744
Loading
Loading
Loading
Loading
+13 −9
Original line number Diff line number Diff line
@@ -1121,15 +1121,19 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
    int head = 0;
    int tail = 0;

    int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_write_zeroes,
                                        BDRV_REQUEST_MAX_SECTORS);
    if (bs->bl.write_zeroes_alignment) {
        assert(is_power_of_2(bs->bl.write_zeroes_alignment));
        head = sector_num & (bs->bl.write_zeroes_alignment - 1);
        tail = (sector_num + nb_sectors) & (bs->bl.write_zeroes_alignment - 1);
        max_write_zeroes &= ~(bs->bl.write_zeroes_alignment - 1);
    int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
    int write_zeroes_sector_align =
        bs->bl.pwrite_zeroes_alignment >> BDRV_SECTOR_BITS;

    max_write_zeroes >>= BDRV_SECTOR_BITS;
    if (write_zeroes_sector_align) {
        assert(is_power_of_2(bs->bl.pwrite_zeroes_alignment));
        head = sector_num & (write_zeroes_sector_align - 1);
        tail = (sector_num + nb_sectors) & (write_zeroes_sector_align - 1);
        max_write_zeroes &= ~(write_zeroes_sector_align - 1);
    }

    assert(nb_sectors <= BDRV_REQUEST_MAX_SECTORS);
    while (nb_sectors > 0 && !ret) {
        int num = nb_sectors;

@@ -1139,9 +1143,9 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
         */
        if (head) {
            /* Make a small request up to the first aligned sector.  */
            num = MIN(nb_sectors, bs->bl.write_zeroes_alignment - head);
            num = MIN(nb_sectors, write_zeroes_sector_align - head);
            head = 0;
        } else if (tail && num > bs->bl.write_zeroes_alignment) {
        } else if (tail && num > write_zeroes_sector_align) {
            /* Shorten the request to the last aligned sector.  */
            num -= tail;
        }
+6 −7
Original line number Diff line number Diff line
@@ -1715,16 +1715,15 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
        bs->bl.discard_alignment = iscsilun->block_size >> BDRV_SECTOR_BITS;
    }

    if (iscsilun->bl.max_ws_len < 0xffffffff) {
        bs->bl.max_write_zeroes =
            sector_limits_lun2qemu(iscsilun->bl.max_ws_len, iscsilun);
    if (iscsilun->bl.max_ws_len < 0xffffffff / iscsilun->block_size) {
        bs->bl.max_pwrite_zeroes =
            iscsilun->bl.max_ws_len * iscsilun->block_size;
    }
    if (iscsilun->lbp.lbpws) {
        bs->bl.write_zeroes_alignment =
            sector_limits_lun2qemu(iscsilun->bl.opt_unmap_gran, iscsilun);
        bs->bl.pwrite_zeroes_alignment =
            iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
    } else {
        bs->bl.write_zeroes_alignment =
            iscsilun->block_size >> BDRV_SECTOR_BITS;
        bs->bl.pwrite_zeroes_alignment = iscsilun->block_size;
    }
    bs->bl.opt_transfer_length =
        sector_limits_lun2qemu(iscsilun->bl.opt_xfer_len, iscsilun);
+1 −1
Original line number Diff line number Diff line
@@ -1193,7 +1193,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
{
    BDRVQcow2State *s = bs->opaque;

    bs->bl.write_zeroes_alignment = s->cluster_sectors;
    bs->bl.pwrite_zeroes_alignment = s->cluster_size;
}

static int qcow2_set_key(BlockDriverState *bs, const char *key)
+1 −1
Original line number Diff line number Diff line
@@ -517,7 +517,7 @@ static void bdrv_qed_refresh_limits(BlockDriverState *bs, Error **errp)
{
    BDRVQEDState *s = bs->opaque;

    bs->bl.write_zeroes_alignment = s->header.cluster_size >> BDRV_SECTOR_BITS;
    bs->bl.pwrite_zeroes_alignment = s->header.cluster_size;
}

/* We have nothing to do for QED reopen, stubs just return
+3 −3
Original line number Diff line number Diff line
@@ -997,9 +997,9 @@ static void vmdk_refresh_limits(BlockDriverState *bs, Error **errp)

    for (i = 0; i < s->num_extents; i++) {
        if (!s->extents[i].flat) {
            bs->bl.write_zeroes_alignment =
                MAX(bs->bl.write_zeroes_alignment,
                    s->extents[i].cluster_sectors);
            bs->bl.pwrite_zeroes_alignment =
                MAX(bs->bl.pwrite_zeroes_alignment,
                    s->extents[i].cluster_sectors << BDRV_SECTOR_BITS);
        }
    }
}
Loading