[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] virtio-blk: trivial code optimization
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3] virtio-blk: trivial code optimization |
Date: |
Mon, 9 Nov 2015 13:57:22 +0000 |
User-agent: |
Mutt/1.5.23 (2015-06-09) |
On Mon, Nov 09, 2015 at 05:03:30PM +0800, address@hidden wrote:
> From: Gonglei <address@hidden>
>
> 1. avoid possible superflous checking
> 2. make code more robustness
>
> Signed-off-by: Gonglei <address@hidden>
> Reviewed-by: Fam Zheng <address@hidden>
> ---
> v3: change the third condition too [Paolo]
> add Fam's R-by
> ---
> hw/block/virtio-blk.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 093e475..9124358 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -404,24 +404,15 @@ void virtio_blk_submit_multireq(BlockBackend *blk,
> MultiReqBuffer *mrb)
> for (i = 0; i < mrb->num_reqs; i++) {
> VirtIOBlockReq *req = mrb->reqs[i];
> if (num_reqs > 0) {
> - bool merge = true;
> -
> - /* merge would exceed maximum number of IOVs */
> - if (niov + req->qiov.niov > IOV_MAX) {
> - merge = false;
> - }
> -
> - /* merge would exceed maximum transfer length of backend device
> */
> - if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors >
> max_xfer_len) {
> - merge = false;
> - }
> -
> - /* requests are not sequential */
> - if (sector_num + nb_sectors != req->sector_num) {
> - merge = false;
> - }
> -
> - if (!merge) {
> + /*
> + * NOTE: We cannot merge the requests in below situations:
> + * 1. requests are not sequential
> + * 2. merge would exceed maximum number of IOVs
> + * 3. merge would exceed maximum transfer length of backend
> device
> + */
> + if (sector_num + nb_sectors != req->sector_num ||
> + niov > IOV_MAX - req->qiov.niov ||
> + nb_sectors > max_xfer_len - req->qiov.size /
> BDRV_SECTOR_SIZE) {
nb_sectors - int
max_xfer_len - int
req->qiov.size - size_t
BDRV_SECTOR_SIZE - unsigned long long
Therefore this expression is an int > unsigned long long comparison.
req->qiov.size can be larger than max_xfer_len so this comparison
suffers from underflow. Please check that req->qiov.size <=
max_xfer_len first.
Stefan
signature.asc
Description: PGP signature