From patchwork Tue Jun 14 21:30:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 9177003 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6B2AA604DB for ; Tue, 14 Jun 2016 21:48:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5BFB428236 for ; Tue, 14 Jun 2016 21:48:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5021D28334; Tue, 14 Jun 2016 21:48:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7554628332 for ; Tue, 14 Jun 2016 21:48:49 +0000 (UTC) Received: from localhost ([::1]:38388 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCwCW-00079I-Ir for patchwork-qemu-devel@patchwork.kernel.org; Tue, 14 Jun 2016 17:48:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCvvY-0007E2-Mx for qemu-devel@nongnu.org; Tue, 14 Jun 2016 17:31:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCvvR-0006v3-H5 for qemu-devel@nongnu.org; Tue, 14 Jun 2016 17:31:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49733) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCvvL-0006qQ-0w; Tue, 14 Jun 2016 17:31:03 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 93E4085547; Tue, 14 Jun 2016 21:31:02 +0000 (UTC) Received: from red.redhat.com (ovpn-116-106.phx2.redhat.com [10.3.116.106]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5ELUl7M019211; Tue, 14 Jun 2016 17:31:01 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Tue, 14 Jun 2016 15:30:36 -0600 Message-Id: <1465939839-30097-15-git-send-email-eblake@redhat.com> In-Reply-To: <1465939839-30097-1-git-send-email-eblake@redhat.com> References: <1465939839-30097-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 14 Jun 2016 21:31:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 14/17] block: Switch transfer length bounds to byte-based X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, Fam Zheng , Stefan Hajnoczi , qemu-block@nongnu.org, "Michael S. Tsirkin" , Peter Lieven , Max Reitz , Ronnie Sahlberg , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Sector-based limits are awkward to think about; in our on-going quest to move to byte-based interfaces, convert max_transfer_length and opt_transfer_length. Rename them (dropping the _length suffix) so that the compiler will help us catch the change in semantics across any rebased code, and improve the documentation. Use unsigned values, so that we don't have to worry about negative values and so that bit-twiddling is easier; however, we are still constrained by 2^31 of signed int in most APIs. Signed-off-by: Eric Blake Reviewed-by: Fam Zheng --- v2: Hoist unrelated cleanups earlier, use name 'max_transfer' in more places, tweak iscsi calculations --- include/block/block_int.h | 11 +++++++---- include/sysemu/block-backend.h | 2 +- block/block-backend.c | 10 +++++----- block/io.c | 23 +++++++++++------------ block/iscsi.c | 20 ++++++++++++-------- block/nbd.c | 2 +- block/raw-posix.c | 3 ++- hw/block/virtio-blk.c | 9 +++++---- hw/scsi/scsi-generic.c | 11 ++++++----- qemu-img.c | 8 ++++---- 10 files changed, 54 insertions(+), 45 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 16c43e2..2b45d57 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -338,11 +338,14 @@ typedef struct BlockLimits { * power of 2, and less than max_pwrite_zeroes if that is set */ uint32_t pwrite_zeroes_alignment; - /* optimal transfer length in sectors */ - int opt_transfer_length; + /* optimal transfer length in bytes (must be power of 2, and + * multiple of bs->request_alignment), or 0 if no preferred size */ + uint32_t opt_transfer; - /* maximal transfer length in sectors */ - int max_transfer_length; + /* maximal transfer length in bytes (need not be power of 2, but + * should be multiple of opt_transfer), or 0 for no 32-bit limit. + * For now, anything larger than INT_MAX is clamped down. */ + uint32_t max_transfer; /* memory alignment so that no bounce buffer is needed */ size_t min_mem_alignment; diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index c04af8e..2469a1c 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -170,7 +170,7 @@ bool blk_is_available(BlockBackend *blk); void blk_lock_medium(BlockBackend *blk, bool locked); void blk_eject(BlockBackend *blk, bool eject_flag); int blk_get_flags(BlockBackend *blk); -int blk_get_max_transfer_length(BlockBackend *blk); +uint32_t blk_get_max_transfer(BlockBackend *blk); int blk_get_max_iov(BlockBackend *blk); void blk_set_guest_block_size(BlockBackend *blk, int align); void *blk_try_blockalign(BlockBackend *blk, size_t size); diff --git a/block/block-backend.c b/block/block-backend.c index 1fb070b..e042544 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1303,16 +1303,16 @@ int blk_get_flags(BlockBackend *blk) } } -/* Returns the maximum transfer length, in sectors; guaranteed nonzero */ -int blk_get_max_transfer_length(BlockBackend *blk) +/* Returns the maximum transfer length, in bytes; guaranteed nonzero */ +uint32_t blk_get_max_transfer(BlockBackend *blk) { BlockDriverState *bs = blk_bs(blk); - int max = 0; + uint32_t max = 0; if (bs) { - max = bs->bl.max_transfer_length; + max = bs->bl.max_transfer; } - return MIN_NON_ZERO(max, BDRV_REQUEST_MAX_SECTORS); + return MIN_NON_ZERO(max, INT_MAX); } int blk_get_max_iov(BlockBackend *blk) diff --git a/block/io.c b/block/io.c index c425ce8..5e38ab4 100644 --- a/block/io.c +++ b/block/io.c @@ -88,8 +88,8 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) error_propagate(errp, local_err); return; } - bs->bl.opt_transfer_length = bs->file->bs->bl.opt_transfer_length; - bs->bl.max_transfer_length = bs->file->bs->bl.max_transfer_length; + bs->bl.opt_transfer = bs->file->bs->bl.opt_transfer; + bs->bl.max_transfer = bs->file->bs->bl.max_transfer; bs->bl.min_mem_alignment = bs->file->bs->bl.min_mem_alignment; bs->bl.opt_mem_alignment = bs->file->bs->bl.opt_mem_alignment; bs->bl.max_iov = bs->file->bs->bl.max_iov; @@ -107,12 +107,10 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) error_propagate(errp, local_err); return; } - bs->bl.opt_transfer_length = - MAX(bs->bl.opt_transfer_length, - bs->backing->bs->bl.opt_transfer_length); - bs->bl.max_transfer_length = - MIN_NON_ZERO(bs->bl.max_transfer_length, - bs->backing->bs->bl.max_transfer_length); + bs->bl.opt_transfer = MAX(bs->bl.opt_transfer, + bs->backing->bs->bl.opt_transfer); + bs->bl.max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, + bs->backing->bs->bl.max_transfer); bs->bl.opt_mem_alignment = MAX(bs->bl.opt_mem_alignment, bs->backing->bs->bl.opt_mem_alignment); @@ -1144,7 +1142,8 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num, return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, 0); } -#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768 +/* Maximum buffer for write zeroes fallback, in bytes */ +#define MAX_WRITE_ZEROES_BOUNCE_BUFFER (32768 << BDRV_SECTOR_BITS) static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int count, BdrvRequestFlags flags) @@ -1202,7 +1201,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, if (ret == -ENOTSUP) { /* Fall back to bounce buffer if write zeroes is unsupported */ - int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length, + int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_WRITE_ZEROES_BOUNCE_BUFFER); BdrvRequestFlags write_flags = flags & ~BDRV_REQ_ZERO_WRITE; @@ -1213,7 +1212,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, write_flags &= ~BDRV_REQ_FUA; need_flush = true; } - num = MIN(num, max_xfer_len << BDRV_SECTOR_BITS); + num = MIN(num, max_transfer); iov.iov_len = num; if (iov.iov_base == NULL) { iov.iov_base = qemu_try_blockalign(bs, num); @@ -1230,7 +1229,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, /* Keep bounce buffer around if it is big enough for all * all future requests. */ - if (num < max_xfer_len << BDRV_SECTOR_BITS) { + if (num < max_transfer) { qemu_vfree(iov.iov_base); iov.iov_base = NULL; } diff --git a/block/iscsi.c b/block/iscsi.c index b4661c9..739d5ca 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -473,9 +473,10 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors, return -EINVAL; } - if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length) { + if (bs->bl.max_transfer && + nb_sectors << BDRV_SECTOR_BITS > bs->bl.max_transfer) { error_report("iSCSI Error: Write of %d sectors exceeds max_xfer_len " - "of %d sectors", nb_sectors, bs->bl.max_transfer_length); + "of %" PRIu32 " bytes", nb_sectors, bs->bl.max_transfer); return -EINVAL; } @@ -650,9 +651,10 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, return -EINVAL; } - if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length) { + if (bs->bl.max_transfer && + nb_sectors << BDRV_SECTOR_BITS > bs->bl.max_transfer) { error_report("iSCSI Error: Read of %d sectors exceeds max_xfer_len " - "of %d sectors", nb_sectors, bs->bl.max_transfer_length); + "of %" PRIu32 " bytes", nb_sectors, bs->bl.max_transfer); return -EINVAL; } @@ -1707,7 +1709,7 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) * iscsi_open(): iscsi targets don't change their limits. */ IscsiLun *iscsilun = bs->opaque; - uint32_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff; + uint64_t max_xfer_len = iscsilun->use_16_for_rw ? 0xffffffff : 0xffff; bs->request_alignment = iscsilun->block_size; @@ -1715,7 +1717,9 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) max_xfer_len = MIN(max_xfer_len, iscsilun->bl.max_xfer_len); } - bs->bl.max_transfer_length = sector_limits_lun2qemu(max_xfer_len, iscsilun); + if (max_xfer_len * iscsilun->block_size < INT_MAX) { + bs->bl.max_transfer = max_xfer_len * iscsilun->block_size; + } if (iscsilun->lbp.lbpu) { if (iscsilun->bl.max_unmap < 0xffffffff) { @@ -1738,8 +1742,8 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) } else { bs->bl.pwrite_zeroes_alignment = iscsilun->block_size; } - bs->bl.opt_transfer_length = - sector_limits_lun2qemu(iscsilun->bl.opt_xfer_len, iscsilun); + assert(iscsilun->bl.opt_xfer_len < INT_MAX / iscsilun->block_size); + bs->bl.opt_transfer = iscsilun->bl.opt_xfer_len * iscsilun->block_size; } /* Note that this will not re-establish a connection with an iSCSI target - it diff --git a/block/nbd.c b/block/nbd.c index bf67c8a..f5511ea 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -363,7 +363,7 @@ static int nbd_co_flush(BlockDriverState *bs) static void nbd_refresh_limits(BlockDriverState *bs, Error **errp) { bs->bl.max_discard = NBD_MAX_SECTORS; - bs->bl.max_transfer_length = NBD_MAX_SECTORS; + bs->bl.max_transfer = NBD_MAX_BUFFER_SIZE; } static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num, diff --git a/block/raw-posix.c b/block/raw-posix.c index aacf132..f2bea85 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -752,7 +752,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) if (S_ISBLK(st.st_mode)) { int ret = hdev_get_max_transfer_length(s->fd); if (ret >= 0) { - bs->bl.max_transfer_length = ret; + assert(ret <= BDRV_REQUEST_MAX_SECTORS); + bs->bl.max_transfer = ret << BDRV_SECTOR_BITS; } } } diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 1d2792e..5000c27 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -382,7 +382,7 @@ static int multireq_compare(const void *a, const void *b) void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) { int i = 0, start = 0, num_reqs = 0, niov = 0, nb_sectors = 0; - int max_xfer_len; + uint32_t max_transfer; int64_t sector_num = 0; if (mrb->num_reqs == 1) { @@ -391,7 +391,7 @@ void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) return; } - max_xfer_len = blk_get_max_transfer_length(mrb->reqs[0]->dev->blk); + max_transfer = blk_get_max_transfer(mrb->reqs[0]->dev->blk); qsort(mrb->reqs, mrb->num_reqs, sizeof(*mrb->reqs), &multireq_compare); @@ -407,8 +407,9 @@ void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) */ if (sector_num + nb_sectors != req->sector_num || niov > blk_get_max_iov(blk) - req->qiov.niov || - req->qiov.size / BDRV_SECTOR_SIZE > max_xfer_len || - nb_sectors > max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE) { + req->qiov.size > max_transfer || + nb_sectors > (max_transfer - + req->qiov.size) / BDRV_SECTOR_SIZE) { submit_requests(blk, mrb, start, num_reqs, niov); num_reqs = 0; } diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index db04a76..f59ed8f 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -225,13 +225,14 @@ static void scsi_read_complete(void * opaque, int ret) if (s->type == TYPE_DISK && r->req.cmd.buf[0] == INQUIRY && r->req.cmd.buf[2] == 0xb0) { - uint32_t max_xfer_len = blk_get_max_transfer_length(s->conf.blk); + uint32_t max_transfer = + blk_get_max_transfer(s->conf.blk) >> BDRV_SECTOR_BITS; - assert(max_xfer_len); - stl_be_p(&r->buf[8], max_xfer_len); + assert(max_transfer); + stl_be_p(&r->buf[8], max_transfer); /* Also take care of the opt xfer len. */ - if (ldl_be_p(&r->buf[12]) > max_xfer_len) { - stl_be_p(&r->buf[12], max_xfer_len); + if (ldl_be_p(&r->buf[12]) > max_transfer) { + stl_be_p(&r->buf[12], max_transfer); } } scsi_req_data(&r->req, len); diff --git a/qemu-img.c b/qemu-img.c index 14e2661..cf9876d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2078,13 +2078,13 @@ static int img_convert(int argc, char **argv) } out_bs = blk_bs(out_blk); - /* increase bufsectors from the default 4096 (2M) if opt_transfer_length + /* increase bufsectors from the default 4096 (2M) if opt_transfer * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB) * as maximum. */ bufsectors = MIN(32768, - MAX(bufsectors, MAX(out_bs->bl.opt_transfer_length, - out_bs->bl.discard_alignment)) - ); + MAX(bufsectors, + MAX(out_bs->bl.opt_transfer >> BDRV_SECTOR_BITS, + out_bs->bl.discard_alignment))); if (skip_create) { int64_t output_sectors = blk_nb_sectors(out_blk);