Re: [PATCH 3/4] vhost: Improve vhost_get_avail_head()

2024-04-25 Thread Gavin Shan

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-...@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 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202404260448.g7f06v7m-...@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




Re: [PATCH 3/4] vhost: Improve vhost_get_avail_head()

2024-04-25 Thread kernel test robot
Hi Gavin,

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-...@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 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202404260448.g7f06v7m-...@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,
  2591struct iovec iov[], unsigned int iov_size,
  2592unsigned int *out_num, unsigned int *in_num,
  2593struct 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;
  2616  
  2617  /* When we start there are none of either input nor output. */
  2618  *out_num = *in_num = 0;
  2619  if (unlikely(log))
  2620  *log_num = 0;
  2621  
  2622  i = head;
  2623  do {
  2624  unsigned iov_count = *in_num + *out_num;
  2625  if (unlikely(i >= vq->num)) {
  2626  vq_err(vq, "Desc index is %u > %u, head = %u",
  2627 i, vq->num, head);
  2628  return -EINVAL;
  2629  }
  2630  if (unlikely(++found > vq->num)) {
  2631  vq_err(vq, "Loop detected: last one at %u "
  2632 "vq size %u head %u\n",
  2633 i, vq->num, head);
  2634  return -EINVAL;
  2635  }
  2636  ret = vhost_get_desc(vq, , i);
  2637  if (unlikely(ret)) {
  2638  vq_err(vq, "Failed to get descriptor: idx %d 
addr %p\n",
  2639 i, vq->desc + i);
  2640  return -EFAULT;
  2641  }
  2642  if (desc.flags & cpu_to_vhost16(vq, 
VRING_DESC_F_INDIRECT)) {
  2643  ret = get_indirect(vq, iov, iov_size,
  2644 out_num, in_num,
  2645 log, log_num, );
  2646  if (unlikely(ret < 0)) {
  2647  if (ret != -EAGAIN)
  2648  vq_err(vq, "Failure detected "
  2649