tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
date: 1 year, 6 months ago
config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/[email protected]/reproduce)
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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/block/virtio_blk.c: In function 'init_vq':
>> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
| ^~
drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
| ^~~~~~~~~~~~~
drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +570 drivers/block/virtio_blk.c
511
512 static int init_vq(struct virtio_blk *vblk)
513 {
514 int err;
515 int i;
516 vq_callback_t **callbacks;
517 const char **names;
518 struct virtqueue **vqs;
519 unsigned short num_vqs;
520 unsigned int num_poll_vqs;
521 struct virtio_device *vdev = vblk->vdev;
522 struct irq_affinity desc = { 0, };
523
524 err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
525 struct virtio_blk_config, num_queues,
526 &num_vqs);
527 if (err)
528 num_vqs = 1;
529
530 if (!err && !num_vqs) {
531 dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
532 return -EINVAL;
533 }
534
535 num_vqs = min_t(unsigned int,
536 min_not_zero(num_request_queues, nr_cpu_ids),
537 num_vqs);
538
539 num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
540
541 vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
542 vblk->io_queues[HCTX_TYPE_READ] = 0;
543 vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
544
545 dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
546 vblk->io_queues[HCTX_TYPE_DEFAULT],
547 vblk->io_queues[HCTX_TYPE_READ],
548 vblk->io_queues[HCTX_TYPE_POLL]);
549
550 vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
551 if (!vblk->vqs)
552 return -ENOMEM;
553
554 names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
555 callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
556 vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
557 if (!names || !callbacks || !vqs) {
558 err = -ENOMEM;
559 goto out;
560 }
561
562 for (i = 0; i < num_vqs - num_poll_vqs; i++) {
563 callbacks[i] = virtblk_done;
564 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
565 names[i] = vblk->vqs[i].name;
566 }
567
568 for (; i < num_vqs; i++) {
569 callbacks[i] = NULL;
> 570 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
571 names[i] = vblk->vqs[i].name;
572 }
573
574 /* Discover virtqueues and write information to configuration. */
575 err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
576 if (err)
577 goto out;
578
579 for (i = 0; i < num_vqs; i++) {
580 spin_lock_init(&vblk->vqs[i].lock);
581 vblk->vqs[i].vq = vqs[i];
582 }
583 vblk->num_vqs = num_vqs;
584
585 out:
586 kfree(vqs);
587 kfree(callbacks);
588 kfree(names);
589 if (err)
590 kfree(vblk->vqs);
591 return err;
592 }
593
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, Dec 04, 2023 at 04:56:35PM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
> commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
> date: 1 year, 6 months ago
> config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/[email protected]/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/[email protected]/reproduce)
>
> 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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> drivers/block/virtio_blk.c: In function 'init_vq':
> >> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
> 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> | ^~
> drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
> 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> | ^~~~~~~~~~~~~
> drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
> 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> vim +570 drivers/block/virtio_blk.c
>
> 511
> 512 static int init_vq(struct virtio_blk *vblk)
> 513 {
> 514 int err;
> 515 int i;
> 516 vq_callback_t **callbacks;
> 517 const char **names;
> 518 struct virtqueue **vqs;
> 519 unsigned short num_vqs;
> 520 unsigned int num_poll_vqs;
> 521 struct virtio_device *vdev = vblk->vdev;
> 522 struct irq_affinity desc = { 0, };
> 523
> 524 err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> 525 struct virtio_blk_config, num_queues,
> 526 &num_vqs);
> 527 if (err)
> 528 num_vqs = 1;
> 529
> 530 if (!err && !num_vqs) {
> 531 dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
> 532 return -EINVAL;
> 533 }
> 534
> 535 num_vqs = min_t(unsigned int,
> 536 min_not_zero(num_request_queues, nr_cpu_ids),
> 537 num_vqs);
> 538
> 539 num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
> 540
> 541 vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
> 542 vblk->io_queues[HCTX_TYPE_READ] = 0;
> 543 vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
> 544
> 545 dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
> 546 vblk->io_queues[HCTX_TYPE_DEFAULT],
> 547 vblk->io_queues[HCTX_TYPE_READ],
> 548 vblk->io_queues[HCTX_TYPE_POLL]);
> 549
> 550 vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
> 551 if (!vblk->vqs)
> 552 return -ENOMEM;
> 553
> 554 names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
> 555 callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
> 556 vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
> 557 if (!names || !callbacks || !vqs) {
> 558 err = -ENOMEM;
> 559 goto out;
> 560 }
> 561
> 562 for (i = 0; i < num_vqs - num_poll_vqs; i++) {
> 563 callbacks[i] = virtblk_done;
> 564 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
> 565 names[i] = vblk->vqs[i].name;
> 566 }
> 567
> 568 for (; i < num_vqs; i++) {
> 569 callbacks[i] = NULL;
> > 570 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> 571 names[i] = vblk->vqs[i].name;
> 572 }
> 573
> 574 /* Discover virtqueues and write information to configuration. */
> 575 err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
> 576 if (err)
> 577 goto out;
> 578
> 579 for (i = 0; i < num_vqs; i++) {
> 580 spin_lock_init(&vblk->vqs[i].lock);
> 581 vblk->vqs[i].vq = vqs[i];
> 582 }
> 583 vblk->num_vqs = num_vqs;
> 584
> 585 out:
> 586 kfree(vqs);
> 587 kfree(callbacks);
> 588 kfree(names);
> 589 if (err)
> 590 kfree(vblk->vqs);
> 591 return err;
> 592 }
> 593
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Stefan, Paolo,
It's a false positive but do we want to fix it? Make i unsigned?
--
MST
On Mon, Dec 04, 2023 at 04:02:07AM -0500, Michael S. Tsirkin wrote:
> On Mon, Dec 04, 2023 at 04:56:35PM +0800, kernel test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
> > commit: 4e0400525691d0e676dbe002641f9a61261f1e1b virtio-blk: support polling I/O
> > date: 1 year, 6 months ago
> > config: x86_64-buildonly-randconfig-006-20230906 (https://download.01.org/0day-ci/archive/20231204/[email protected]/config)
> > compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/[email protected]/reproduce)
> >
> > 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 <[email protected]>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >
> > All warnings (new ones prefixed by >>):
> >
> > drivers/block/virtio_blk.c: In function 'init_vq':
> > >> drivers/block/virtio_blk.c:570:68: warning: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 7 [-Wformat-truncation=]
> > 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> > | ^~
> > drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
> > 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> > | ^~~~~~~~~~~~~
> > drivers/block/virtio_blk.c:570:17: note: 'snprintf' output between 11 and 21 bytes into a destination of size 16
> > 570 | snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> >
> > vim +570 drivers/block/virtio_blk.c
> >
> > 511
> > 512 static int init_vq(struct virtio_blk *vblk)
> > 513 {
> > 514 int err;
> > 515 int i;
> > 516 vq_callback_t **callbacks;
> > 517 const char **names;
> > 518 struct virtqueue **vqs;
> > 519 unsigned short num_vqs;
> > 520 unsigned int num_poll_vqs;
> > 521 struct virtio_device *vdev = vblk->vdev;
> > 522 struct irq_affinity desc = { 0, };
> > 523
> > 524 err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> > 525 struct virtio_blk_config, num_queues,
> > 526 &num_vqs);
> > 527 if (err)
> > 528 num_vqs = 1;
> > 529
> > 530 if (!err && !num_vqs) {
> > 531 dev_err(&vdev->dev, "MQ advertised but zero queues reported\n");
> > 532 return -EINVAL;
> > 533 }
> > 534
> > 535 num_vqs = min_t(unsigned int,
> > 536 min_not_zero(num_request_queues, nr_cpu_ids),
> > 537 num_vqs);
> > 538
> > 539 num_poll_vqs = min_t(unsigned int, poll_queues, num_vqs - 1);
> > 540
> > 541 vblk->io_queues[HCTX_TYPE_DEFAULT] = num_vqs - num_poll_vqs;
> > 542 vblk->io_queues[HCTX_TYPE_READ] = 0;
> > 543 vblk->io_queues[HCTX_TYPE_POLL] = num_poll_vqs;
> > 544
> > 545 dev_info(&vdev->dev, "%d/%d/%d default/read/poll queues\n",
> > 546 vblk->io_queues[HCTX_TYPE_DEFAULT],
> > 547 vblk->io_queues[HCTX_TYPE_READ],
> > 548 vblk->io_queues[HCTX_TYPE_POLL]);
> > 549
> > 550 vblk->vqs = kmalloc_array(num_vqs, sizeof(*vblk->vqs), GFP_KERNEL);
> > 551 if (!vblk->vqs)
> > 552 return -ENOMEM;
> > 553
> > 554 names = kmalloc_array(num_vqs, sizeof(*names), GFP_KERNEL);
> > 555 callbacks = kmalloc_array(num_vqs, sizeof(*callbacks), GFP_KERNEL);
> > 556 vqs = kmalloc_array(num_vqs, sizeof(*vqs), GFP_KERNEL);
> > 557 if (!names || !callbacks || !vqs) {
> > 558 err = -ENOMEM;
> > 559 goto out;
> > 560 }
> > 561
> > 562 for (i = 0; i < num_vqs - num_poll_vqs; i++) {
> > 563 callbacks[i] = virtblk_done;
> > 564 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req.%d", i);
> > 565 names[i] = vblk->vqs[i].name;
> > 566 }
> > 567
> > 568 for (; i < num_vqs; i++) {
> > 569 callbacks[i] = NULL;
> > > 570 snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
> > 571 names[i] = vblk->vqs[i].name;
> > 572 }
> > 573
> > 574 /* Discover virtqueues and write information to configuration. */
> > 575 err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
> > 576 if (err)
> > 577 goto out;
> > 578
> > 579 for (i = 0; i < num_vqs; i++) {
> > 580 spin_lock_init(&vblk->vqs[i].lock);
> > 581 vblk->vqs[i].vq = vqs[i];
> > 582 }
> > 583 vblk->num_vqs = num_vqs;
> > 584
> > 585 out:
> > 586 kfree(vqs);
> > 587 kfree(callbacks);
> > 588 kfree(names);
> > 589 if (err)
> > 590 kfree(vblk->vqs);
> > 591 return err;
> > 592 }
> > 593
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests/wiki
>
> Stefan, Paolo,
> It's a false positive but do we want to fix it? Make i unsigned?
It's a false positive:
That maximum number of virtqueues is 65535. That's 5 characters. The
maximum value of num_poll_vqs is actually 65534 because it's capped at
num_vqs - 1.
The warning reveals that the analysis thinks i can be negative:
drivers/block/virtio_blk.c:570:58: note: directive argument in the range [-2147483648, 65534]
It can't because of num_poll_vqs = min_t(unsigned int, poll_queues,
num_vqs - 1).
VQ_NAME_LEN is 16, so 9 bytes are used for "req_poll." and the NUL
terminator:
snprintf(vblk->vqs[i].name, VQ_NAME_LEN, "req_poll.%d", i);
9 + 5 = 14 is always less than VQ_NAME_LEN (16), so it looks like a
false positive.
That said, it's still worth cleaning up the types because they are
inconsistent:
unsigned short num_vqs;
unsigned int num_poll_vqs;
and using "%d" for an unsigned value also contributes to the problem.
I'll send a patch.
Stefan