Re: [PATCH RFC v4 5/5] misc: Add edu_srpoc surprise removal POC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] misc: edu_srpoc: lost disconnect_work completion causes remove() deadlock
--

commit bc425826c0abe82189b4fc145666a7bddb680acf
Author: Abhin Parekadan Jose <abhinjoses@gmail.com>

misc: Add edu_srpoc surprise removal POC driver

A test driver for the QEMU edu device that reproduces a surprise removal hang.
It blocks in remove() until a delayed interrupt arrives or the device is
surprise removed and its disconnect work runs.

Link: https://lore.kernel.org/all/20260905183905.997833-4-abhinjoses@gmail.com/
Link: https://lore.kernel.org/all/20260905185027.291191F00A3A@smtp.kernel.org/
Link: https://lore.kernel.org/all/20260927175203.928270-6-abhinjoses@gmail.com/
Link: https://lore.kernel.org/all/20260927175937.E006F1F00893@smtp.kernel.org/

> diff --git a/drivers/misc/edu_srpoc.c b/drivers/misc/edu_srpoc.c
> new file mode 100644
> index 0000000000000..1f395b2c31ee3
> --- /dev/null
> +++ b/drivers/misc/edu_srpoc.c

[ ... ]

> +static void edu_remove(struct pci_dev *pdev)
> +{
> +	struct edu_dev *edu = pci_get_drvdata(pdev);
> +
> +	reinit_completion(&edu->irq_done);

[Severity: Medium]
Can this cause a deadlock if the device is surprise-removed before
edu_remove() starts executing?

If the PCI core schedules and executes edu_disconnect() first, it clears the
disconnect_work_enable flag and signals complete() on edu->irq_done:

drivers/misc/edu_srpoc.c:edu_disconnect() {
    ...
	if (!pci_test_and_clear_disconnect_enable(pdev))
		return;
    ...
	complete(&edu->irq_done);
}

When edu_remove() then runs, it unconditionally calls reinit_completion(),
which wipes out the unblock signal from edu_disconnect(). Because the
disconnect flag was already cleared, edu_disconnect() will not run again.

> +	iowrite32(EDU_STATUS_IRQFACT, edu->regs + EDU_REG_STATUS);
> +	iowrite32(600000, edu->regs + EDU_REG_DELAYED_IRQ);
> +
> +	dev_info(&pdev->dev, "Waiting for IRQ in remove()\n");
> +	wait_for_completion(&edu->irq_done);

Since the device is gone and no hardware interrupts will arrive, will
wait_for_completion() block indefinitely here, defeating the test driver's
stated purpose of unblocking on surprise removal?

> +	dev_info(&pdev->dev, "Unblocked, cleaning up\n");
> +
> +	pci_clear_disconnect_work(pdev);
> +	free_irq(pci_irq_vector(pdev, 0), edu);
> +	pci_free_irq_vectors(pdev);
> +	pci_clear_master(pdev);
> +	pci_iounmap(pdev, edu->regs);
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927182017.938565-1-abhinjoses@gmail.com?part=5