[dpdk-dev] [PATCH v1] igb_uio: fix unexpected removal for hot-unplug
Jeff Guo
jia.guo at intel.com
Fri Oct 19 10:35:46 CEST 2018
On 10/19/2018 12:06 AM, Ferruh Yigit wrote:
> On 10/18/2018 7:27 AM, Jeff Guo wrote:
>> When a device is hot-unplugged, pci_remove will be invoked unexpectedly
>> before pci_release, it will caused kernel hung issue which will throw the
>> error info of "Trying to free already-free IRQ XXX". And on the other hand,
>> if pci_remove before pci_release, the interrupt will not got chance to be
>> disabled. So this patch aim to fix this issue by adding pci_release call
>> in pci_remove, it will gurranty that all pci clean up will be done before
>> pci removal.
>>
>> Signed-off-by: Jeff Guo <jia.guo at intel.com>
>> ---
>> kernel/linux/igb_uio/igb_uio.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
>> index fede66c..3cf394b 100644
>> --- a/kernel/linux/igb_uio/igb_uio.c
>> +++ b/kernel/linux/igb_uio/igb_uio.c
>> @@ -570,6 +570,8 @@ igbuio_pci_remove(struct pci_dev *dev)
>> {
>> struct rte_uio_pci_dev *udev = pci_get_drvdata(dev);
>>
>> + igbuio_pci_release(&udev->info, NULL);
>> +
> Hi Jeff,
>
> This is simpler approach comparing to previous version.
>
> And do you know if igbuio_pci_release() won't be called after
> igbuio_pci_remove() because that will also cause crash, and indeed it will cause
> a crash in the uio too.
>
> The flow as far as I can see:
> when uioN device opened by application, igbuio_pci_open() is called.
>
> If device removed, I expect driver remove() function called, which has a call
> stack like below:
>
> igbuio_pci_remove()
> uio_unregister_device()
> uio_device_release()
> kfree(struct uio_device)
>
> After this point udev is freed and igbuio_pci_release() shouldn't be called, so
> I assume uioN device closed before this point but I couldn't find where, if not
> closed, closing it later will crash.
What i saw is that after igb_uio remove , if detach the device the pci
release will be called, so the
igbuo_pci_release should be called again.
> I can't test the hotplug case, can you please confirm above patch fixing crashes
> you observed for your use cases?
yes, it could be fix the crashed i observed right now.
>
> And for regular usecase this change shouldn't cause any problem, so at worst it
> may not be fixing all hotplug issues, which looks safe to get.
I think it would fix this hung issue that caused of double free irq and
would not have side effect anyway.
More information about the dev
mailing list