Received-SPF: pass (google.com: domain of linux-kernel+bounces-159335-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <9237d933-406d-4120-a5fd-34a238f2ccdb@redhat.com>
Date: Fri, 26 Apr 2024 08:45:44 +1000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/4] vhost: Improve vhost_get_avail_head()
Content-Language: en-US
To: kernel test robot <lkp@intel.com>, virtualization@lists.linux.dev
Cc: oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org,
 mst@redhat.com, jasowang@redhat.com, shan.gavin@gmail.com
References: <20240423032407.262329-4-gshan@redhat.com>
 <202404260448.g7F06v7M-lkp@intel.com>
From: Gavin Shan <gshan@redhat.com>
In-Reply-To: <202404260448.g7F06v7M-lkp@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/26/24 06:42, kernel test robot wrote:> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on mst-vhost/linux-next]
> [also build test WARNING on linus/master v6.9-rc5 next-20240424]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Gavin-Shan/vhost-Drop-variable-last_avail_idx-in-vhost_get_vq_desc/20240423-112803
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
> patch link:    https://lore.kernel.org/r/20240423032407.262329-4-gshan%40redhat.com
> patch subject: [PATCH 3/4] vhost: Improve vhost_get_avail_head()
> config: i386-randconfig-141-20240426 (https://download.01.org/0day-ci/archive/20240426/202404260448.g7F06v7M-lkp@intel.com/config)
> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202404260448.g7F06v7M-lkp@intel.com/
> 
> smatch warnings:
> drivers/vhost/vhost.c:2614 vhost_get_vq_desc() warn: unsigned 'head' is never less than zero.
> drivers/vhost/vhost.c:2614 vhost_get_vq_desc() warn: error code type promoted to positive: 'head'
> 
> vim +/head +2614 drivers/vhost/vhost.c
> 
>    2581	
>    2582	/* This looks in the virtqueue and for the first available buffer, and converts
>    2583	 * it to an iovec for convenient access.  Since descriptors consist of some
>    2584	 * number of output then some number of input descriptors, it's actually two
>    2585	 * iovecs, but we pack them into one and note how many of each there were.
>    2586	 *
>    2587	 * This function returns the descriptor number found, or vq->num (which is
>    2588	 * never a valid descriptor number) if none was found.  A negative code is
>    2589	 * returned on error. */
>    2590	int vhost_get_vq_desc(struct vhost_virtqueue *vq,
>    2591			      struct iovec iov[], unsigned int iov_size,
>    2592			      unsigned int *out_num, unsigned int *in_num,
>    2593			      struct vhost_log *log, unsigned int *log_num)
>    2594	{
>    2595		struct vring_desc desc;
>    2596		unsigned int i, head, found = 0;
>    2597		int ret, access;
>    2598	
>    2599		if (vq->avail_idx == vq->last_avail_idx) {
>    2600			ret = vhost_get_avail_idx(vq);
>    2601			if (unlikely(ret))
>    2602				return ret;
>    2603	
>    2604			/* If there's nothing new since last we looked, return
>    2605			 * invalid.
>    2606			 */
>    2607			if (vq->avail_idx == vq->last_avail_idx)
>    2608				return vq->num;
>    2609		}
>    2610	
>    2611		/* Grab the next descriptor number they're advertising, and increment
>    2612		 * the index we've seen. */
>    2613		head = vhost_get_avail_head(vq);
>>   2614		if (unlikely(head < 0))
>    2615			return head;

Thanks for the report. @head needs to be 'int' instead of 'unsigned int'
so that it can hold the error number from vhost_get_avail_head(). I would
give it more time to see if there are other review comments before I revise
it to fix it up.

Thanks,
Gavin