[v2] virtio-blk: trivial code optimization

From: Gonglei <arei.gonglei@huawei.com>

1. avoid possible superflous checking
2. make code more robustness

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
v2: address Paolo's comments, thanks.
---
 hw/block/virtio-blk.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

On Mon, 11/09 11:19, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> 1. avoid possible superflous checking
> 2. make code more robustness
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v2: address Paolo's comments, thanks.
> ---
>  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..21f8d72 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 ||
> +                req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
>                  submit_requests(blk, mrb, start, num_reqs, niov);
>                  num_reqs = 0;
>              }
> -- 
> 1.7.12.4
> 
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

On 09/11/2015 04:19, arei.gonglei@huawei.com wrote:
> +             * 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 ||
> +                req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {

Hi Gonglei,

the third condition should also be changed to "new > max - old".

Thanks,

Paolo

On 2015/11/9 16:41, Paolo Bonzini wrote:
> 
> 
> On 09/11/2015 04:19, arei.gonglei@huawei.com wrote:
>> +             * 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 ||
>> +                req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
> 
> Hi Gonglei,
> 
> the third condition should also be changed to "new > max - old".
> 
Okay, will do.  :)

Thanks,
-Gonglei