On 2015/11/9 21:57, Stefan Hajnoczi wrote: > On Mon, Nov 09, 2015 at 05:03:30PM +0800, [email protected] wrote: >> From: Gonglei <[email protected]> >> >> 1. avoid possible superflous checking >> 2. make code more robustness >> >> Signed-off-by: Gonglei <[email protected]> >> Reviewed-by: Fam Zheng <[email protected]> >> --- >> 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. > Sorry, I'm confused. max_xfer_len is int, "req->qiov.size / BDRV_SECTOR_SIZE" is unsigned long long, so, "max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE" will be int,
and nb_sectors is int too, so this comparison is right. Am I wrong? > 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. > Regards, -Gonglei
