On Tue, Nov 10, 2015 at 02:35:19PM +0800, Gonglei wrote: > On 2015/11/9 21:57, Stefan Hajnoczi wrote: > > On Mon, Nov 09, 2015 at 05:03:30PM +0800, arei.gong...@huawei.com wrote: > >> From: Gonglei <arei.gong...@huawei.com> > >> > >> 1. avoid possible superflous checking > >> 2. make code more robustness > >> > >> Signed-off-by: Gonglei <arei.gong...@huawei.com> > >> Reviewed-by: Fam Zheng <f...@redhat.com> > >> --- > >> 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,
The type of "max_xfer_len - req->qiov.size / BDRV_SECTOR_SIZE" cannot be int because you said req->qiov.size / BDRV_SECTOR_SIZE" is unsigned long long. The C99 standard says: 6.3.1.1 Boolean, characters, and integers - The rank of a signed integer type shall be greater than the rank of any signed integer type with less precision. ... - The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any. 6.3.1.8 Usual arithmetic conversions Otherwise, if the operand that has unsigned integer type has rank greater or equal to the rank of the type of the other operand, then the operand with signed integer type is converted to the type of the operand with unsigned integer type. So the max_xfer_len int operand must be converted to the higher ranking unsigned long long. Stefan
signature.asc
Description: PGP signature