Hello Jason Wang,
The patch 030881372460: "vhost_net: basic polling support" from Mar
4, 2016, leads to the following
Smatch static checker warning:
drivers/vhost/vhost.c:2565 vhost_new_msg()
warn: sleeping in atomic context
vers/vhost/net.c
509 static void vhost_net_busy_poll(struct vhost_net *net,
510 struct vhost_virtqueue *rvq,
511 struct vhost_virtqueue *tvq,
512 bool *busyloop_intr,
513 bool poll_rx)
514 {
515 unsigned long busyloop_timeout;
516 unsigned long endtime;
517 struct socket *sock;
518 struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
519
520 /* Try to hold the vq mutex of the paired virtqueue. We
can't
521 * use mutex_lock() here since we could not guarantee a
522 * consistenet lock ordering.
523 */
524 if (!mutex_trylock(&vq->mutex))
525 return;
526
527 vhost_disable_notify(&net->dev, vq);
528 sock = vhost_vq_get_backend(rvq);
529
530 busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
531 tvq->busyloop_timeout;
532
533 preempt_disable();
^^^^^^^^^^^^^^^^^
This bumps the preemp_count.
I guess this is to disable page faults?
534 endtime = busy_clock() + busyloop_timeout;
535
536 while (vhost_can_busy_poll(endtime)) {
537 if (vhost_has_work(&net->dev)) {
538 *busyloop_intr = true;
539 break;
540 }
541
542 if ((sock_has_rx_data(sock) &&
543 !vhost_vq_avail_empty(&net->dev, rvq)) ||
The call tree from here to the GFP_KERNEL is very long...
vhost_vq_avail_empty()
-> vhost_get_avail_idx()
-> __vhost_get_user()
-> __vhost_get_user_slow()
-> translate_desc()
-> vhost_iotlb_miss vhost_new_msg() <-- GFP_KERNEL
544 !vhost_vq_avail_empty(&net->dev, tvq))
545 break;
546
547 cpu_relax();
548 }
549
550 preempt_enable();
551
552 if (poll_rx || sock_has_rx_data(sock))
553 vhost_net_busy_poll_try_queue(net, vq);
regards,
dan carpenter
Hi Dan: On Wed, Aug 11, 2021 at 8:14 PM Dan Carpenter <dan.carpenter at oracle.com> wrote:> > Hello Jason Wang, > > The patch 030881372460: "vhost_net: basic polling support" from Mar > 4, 2016, leads to the following > Smatch static checker warning: > > drivers/vhost/vhost.c:2565 vhost_new_msg() > warn: sleeping in atomic context > > vers/vhost/net.c > 509 static void vhost_net_busy_poll(struct vhost_net *net, > 510 struct vhost_virtqueue *rvq, > 511 struct vhost_virtqueue *tvq, > 512 bool *busyloop_intr, > 513 bool poll_rx) > 514 { > 515 unsigned long busyloop_timeout; > 516 unsigned long endtime; > 517 struct socket *sock; > 518 struct vhost_virtqueue *vq = poll_rx ? tvq : rvq; > 519 > 520 /* Try to hold the vq mutex of the paired virtqueue. We can't > 521 * use mutex_lock() here since we could not guarantee a > 522 * consistenet lock ordering. > 523 */ > 524 if (!mutex_trylock(&vq->mutex)) > 525 return; > 526 > 527 vhost_disable_notify(&net->dev, vq); > 528 sock = vhost_vq_get_backend(rvq); > 529 > 530 busyloop_timeout = poll_rx ? rvq->busyloop_timeout: > 531 tvq->busyloop_timeout; > 532 > 533 preempt_disable(); > ^^^^^^^^^^^^^^^^^ > This bumps the preemp_count. > > I guess this is to disable page faults?No, it's intended since we will use busy_clock() which uses the per cpu variable.> > 534 endtime = busy_clock() + busyloop_timeout; > 535 > 536 while (vhost_can_busy_poll(endtime)) { > 537 if (vhost_has_work(&net->dev)) { > 538 *busyloop_intr = true; > 539 break; > 540 } > 541 > 542 if ((sock_has_rx_data(sock) && > 543 !vhost_vq_avail_empty(&net->dev, rvq)) || > > The call tree from here to the GFP_KERNEL is very long... > > vhost_vq_avail_empty() > -> vhost_get_avail_idx() > -> __vhost_get_user() > -> __vhost_get_user_slow() > -> translate_desc() > -> vhost_iotlb_miss vhost_new_msg() <-- GFP_KERNELThis won't happen in reality since: We will make sure the IOTLB contains the translation for avail idx. See vq_meta_prefetch() that will be called at the begining of handle_tx() and handle_rx(). So it looks like a false positive to me. Thanks> > 544 !vhost_vq_avail_empty(&net->dev, tvq)) > 545 break; > 546 > 547 cpu_relax(); > 548 } > 549 > 550 preempt_enable(); > 551 > 552 if (poll_rx || sock_has_rx_data(sock)) > 553 vhost_net_busy_poll_try_queue(net, vq); > > regards, > dan carpenter >