Skip to content

Commit 3daed63

Browse files
vdasahargregkh
vdasahar
authored and
gregkh
committed
VMCI: Use threaded irqs instead of tasklets
The vmci_dispatch_dgs() tasklet function calls vmci_read_data() which uses wait_event() resulting in invalid sleep in an atomic context (and therefore potentially in a deadlock). Use threaded irqs to fix this issue and completely remove usage of tasklets. [ 20.264639] BUG: sleeping function called from invalid context at drivers/misc/vmw_vmci/vmci_guest.c:145 [ 20.264643] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 762, name: vmtoolsd [ 20.264645] preempt_count: 101, expected: 0 [ 20.264646] RCU nest depth: 0, expected: 0 [ 20.264647] 1 lock held by vmtoolsd/762: [ 20.264648] #0: ffff0000874ae440 (sk_lock-AF_VSOCK){+.+.}-{0:0}, at: vsock_connect+0x60/0x330 [vsock] [ 20.264658] Preemption disabled at: [ 20.264659] [<ffff80000151d7d8>] vmci_send_datagram+0x44/0xa0 [vmw_vmci] [ 20.264665] CPU: 0 PID: 762 Comm: vmtoolsd Not tainted 5.19.0-0.rc8.20220727git39c3c396f813.60.fc37.aarch64 #1 [ 20.264667] Hardware name: VMware, Inc. VBSA/VBSA, BIOS VEFI 12/31/2020 [ 20.264668] Call trace: [ 20.264669] dump_backtrace+0xc4/0x130 [ 20.264672] show_stack+0x24/0x80 [ 20.264673] dump_stack_lvl+0x88/0xb4 [ 20.264676] dump_stack+0x18/0x34 [ 20.264677] __might_resched+0x1a0/0x280 [ 20.264679] __might_sleep+0x58/0x90 [ 20.264681] vmci_read_data+0x74/0x120 [vmw_vmci] [ 20.264683] vmci_dispatch_dgs+0x64/0x204 [vmw_vmci] [ 20.264686] tasklet_action_common.constprop.0+0x13c/0x150 [ 20.264688] tasklet_action+0x40/0x50 [ 20.264689] __do_softirq+0x23c/0x6b4 [ 20.264690] __irq_exit_rcu+0x104/0x214 [ 20.264691] irq_exit_rcu+0x1c/0x50 [ 20.264693] el1_interrupt+0x38/0x6c [ 20.264695] el1h_64_irq_handler+0x18/0x24 [ 20.264696] el1h_64_irq+0x68/0x6c [ 20.264697] preempt_count_sub+0xa4/0xe0 [ 20.264698] _raw_spin_unlock_irqrestore+0x64/0xb0 [ 20.264701] vmci_send_datagram+0x7c/0xa0 [vmw_vmci] [ 20.264703] vmci_datagram_dispatch+0x84/0x100 [vmw_vmci] [ 20.264706] vmci_datagram_send+0x2c/0x40 [vmw_vmci] [ 20.264709] vmci_transport_send_control_pkt+0xb8/0x120 [vmw_vsock_vmci_transport] [ 20.264711] vmci_transport_connect+0x40/0x7c [vmw_vsock_vmci_transport] [ 20.264713] vsock_connect+0x278/0x330 [vsock] [ 20.264715] __sys_connect_file+0x8c/0xc0 [ 20.264718] __sys_connect+0x84/0xb4 [ 20.264720] __arm64_sys_connect+0x2c/0x3c [ 20.264721] invoke_syscall+0x78/0x100 [ 20.264723] el0_svc_common.constprop.0+0x68/0x124 [ 20.264724] do_el0_svc+0x38/0x4c [ 20.264725] el0_svc+0x60/0x180 [ 20.264726] el0t_64_sync_handler+0x11c/0x150 [ 20.264728] el0t_64_sync+0x190/0x194 Signed-off-by: Vishnu Dasa <[email protected]> Suggested-by: Zack Rusin <[email protected]> Reported-by: Nadav Amit <[email protected]> Reported-by: Nathan Chancellor <[email protected]> Tested-by: Nathan Chancellor <[email protected]> Fixes: 463713e ("VMCI: dma dg: add support for DMA datagrams receive") Cc: <[email protected]> # v5.18+ Cc: VMware PV-Drivers Reviewers <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: Bryan Tan <[email protected]> Reviewed-by: Bryan Tan <[email protected]> Reviewed-by: Zack Rusin <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent aaca766 commit 3daed63

File tree

1 file changed

+19
-30
lines changed

1 file changed

+19
-30
lines changed

‎drivers/misc/vmw_vmci/vmci_guest.c‎

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ struct vmci_guest_device {
5656

5757
bool exclusive_vectors;
5858

59-
struct tasklet_struct datagram_tasklet;
60-
struct tasklet_struct bm_tasklet;
6159
struct wait_queue_head inout_wq;
6260

6361
void *data_buffer;
@@ -304,9 +302,8 @@ static int vmci_check_host_caps(struct pci_dev *pdev)
304302
* This function assumes that it has exclusive access to the data
305303
* in register(s) for the duration of the call.
306304
*/
307-
static void vmci_dispatch_dgs(unsigned long data)
305+
static void vmci_dispatch_dgs(struct vmci_guest_device *vmci_dev)
308306
{
309-
struct vmci_guest_device *vmci_dev = (struct vmci_guest_device *)data;
310307
u8 *dg_in_buffer = vmci_dev->data_buffer;
311308
struct vmci_datagram *dg;
312309
size_t dg_in_buffer_size = VMCI_MAX_DG_SIZE;
@@ -465,10 +462,8 @@ static void vmci_dispatch_dgs(unsigned long data)
465462
* Scans the notification bitmap for raised flags, clears them
466463
* and handles the notifications.
467464
*/
468-
static void vmci_process_bitmap(unsigned long data)
465+
static void vmci_process_bitmap(struct vmci_guest_device *dev)
469466
{
470-
struct vmci_guest_device *dev = (struct vmci_guest_device *)data;
471-
472467
if (!dev->notification_bitmap) {
473468
dev_dbg(dev->dev, "No bitmap present in %s\n", __func__);
474469
return;
@@ -486,13 +481,13 @@ static irqreturn_t vmci_interrupt(int irq, void *_dev)
486481
struct vmci_guest_device *dev = _dev;
487482

488483
/*
489-
* If we are using MSI-X with exclusive vectors then we simply schedule
490-
* the datagram tasklet, since we know the interrupt was meant for us.
484+
* If we are using MSI-X with exclusive vectors then we simply call
485+
* vmci_dispatch_dgs(), since we know the interrupt was meant for us.
491486
* Otherwise we must read the ICR to determine what to do.
492487
*/
493488

494489
if (dev->exclusive_vectors) {
495-
tasklet_schedule(&dev->datagram_tasklet);
490+
vmci_dispatch_dgs(dev);
496491
} else {
497492
unsigned int icr;
498493

@@ -502,12 +497,12 @@ static irqreturn_t vmci_interrupt(int irq, void *_dev)
502497
return IRQ_NONE;
503498

504499
if (icr & VMCI_ICR_DATAGRAM) {
505-
tasklet_schedule(&dev->datagram_tasklet);
500+
vmci_dispatch_dgs(dev);
506501
icr &= ~VMCI_ICR_DATAGRAM;
507502
}
508503

509504
if (icr & VMCI_ICR_NOTIFICATION) {
510-
tasklet_schedule(&dev->bm_tasklet);
505+
vmci_process_bitmap(dev);
511506
icr &= ~VMCI_ICR_NOTIFICATION;
512507
}
513508

@@ -536,7 +531,7 @@ static irqreturn_t vmci_interrupt_bm(int irq, void *_dev)
536531
struct vmci_guest_device *dev = _dev;
537532

538533
/* For MSI-X we can just assume it was meant for us. */
539-
tasklet_schedule(&dev->bm_tasklet);
534+
vmci_process_bitmap(dev);
540535

541536
return IRQ_HANDLED;
542537
}
@@ -638,10 +633,6 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
638633
vmci_dev->iobase = iobase;
639634
vmci_dev->mmio_base = mmio_base;
640635

641-
tasklet_init(&vmci_dev->datagram_tasklet,
642-
vmci_dispatch_dgs, (unsigned long)vmci_dev);
643-
tasklet_init(&vmci_dev->bm_tasklet,
644-
vmci_process_bitmap, (unsigned long)vmci_dev);
645636
init_waitqueue_head(&vmci_dev->inout_wq);
646637

647638
if (mmio_base != NULL) {
@@ -808,8 +799,9 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
808799
* Request IRQ for legacy or MSI interrupts, or for first
809800
* MSI-X vector.
810801
*/
811-
error = request_irq(pci_irq_vector(pdev, 0), vmci_interrupt,
812-
IRQF_SHARED, KBUILD_MODNAME, vmci_dev);
802+
error = request_threaded_irq(pci_irq_vector(pdev, 0), NULL,
803+
vmci_interrupt, IRQF_SHARED,
804+
KBUILD_MODNAME, vmci_dev);
813805
if (error) {
814806
dev_err(&pdev->dev, "Irq %u in use: %d\n",
815807
pci_irq_vector(pdev, 0), error);
@@ -823,19 +815,21 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
823815
* between the vectors.
824816
*/
825817
if (vmci_dev->exclusive_vectors) {
826-
error = request_irq(pci_irq_vector(pdev, 1),
827-
vmci_interrupt_bm, 0, KBUILD_MODNAME,
828-
vmci_dev);
818+
error = request_threaded_irq(pci_irq_vector(pdev, 1), NULL,
819+
vmci_interrupt_bm, 0,
820+
KBUILD_MODNAME, vmci_dev);
829821
if (error) {
830822
dev_err(&pdev->dev,
831823
"Failed to allocate irq %u: %d\n",
832824
pci_irq_vector(pdev, 1), error);
833825
goto err_free_irq;
834826
}
835827
if (caps_in_use & VMCI_CAPS_DMA_DATAGRAM) {
836-
error = request_irq(pci_irq_vector(pdev, 2),
837-
vmci_interrupt_dma_datagram,
838-
0, KBUILD_MODNAME, vmci_dev);
828+
error = request_threaded_irq(pci_irq_vector(pdev, 2),
829+
NULL,
830+
vmci_interrupt_dma_datagram,
831+
0, KBUILD_MODNAME,
832+
vmci_dev);
839833
if (error) {
840834
dev_err(&pdev->dev,
841835
"Failed to allocate irq %u: %d\n",
@@ -871,8 +865,6 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
871865

872866
err_free_irq:
873867
free_irq(pci_irq_vector(pdev, 0), vmci_dev);
874-
tasklet_kill(&vmci_dev->datagram_tasklet);
875-
tasklet_kill(&vmci_dev->bm_tasklet);
876868

877869
err_disable_msi:
878870
pci_free_irq_vectors(pdev);
@@ -943,9 +935,6 @@ static void vmci_guest_remove_device(struct pci_dev *pdev)
943935
free_irq(pci_irq_vector(pdev, 0), vmci_dev);
944936
pci_free_irq_vectors(pdev);
945937

946-
tasklet_kill(&vmci_dev->datagram_tasklet);
947-
tasklet_kill(&vmci_dev->bm_tasklet);
948-
949938
if (vmci_dev->notification_bitmap) {
950939
/*
951940
* The device reset above cleared the bitmap state of the

0 commit comments

Comments
 (0)