Message ID | 20240229144723.13047-2-edmund.raile@proton.me |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2097:b0:108:e6aa:91d0 with SMTP id gs23csp447017dyb; Thu, 29 Feb 2024 06:48:35 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWNjDva+FDc5yT4xuFgi0Vw0T8n/wp2rItQAg2ufdSzhMIn3/HZ3KPoMz7yvHEVD/POqAVoPgxOVvaWP7urpAJoC0sQtQ== X-Google-Smtp-Source: AGHT+IFPvRwzR7gx1Q0JssYggl8p0UM6RMldQiYYz+AD+hW4smF54VgW6WnWfKBXjALdrqngE+iy X-Received: by 2002:a05:6a00:2d9b:b0:6e5:2f27:5235 with SMTP id fb27-20020a056a002d9b00b006e52f275235mr3368273pfb.11.1709218114756; Thu, 29 Feb 2024 06:48:34 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709218114; cv=pass; d=google.com; s=arc-20160816; b=un60iioLYI9ZNzPKFYmru20ML31uhb6f7KjwlGuxDp+95G8BnCUU2eNWI+miWGao0y pFPKpYZsqnBWNRLp7F7SKKL5+GQnsq5o4kD3o960XrLRIvHfUQVwxagLvxWcZt/cEBW2 jTlDK5U6YrdJKOZjUsnZ+0kqvNFLsfdC9+CmTj3Fo0FpspaJboYeMELLe+90YUV4joEi jzuDBf2HckFBFoWL/ZbUBVV81yvjTG0Jnv137XxlhW0aN1XDUwB1XCyLuVtiC0I+HV/i Gp2vS/q1VFwTRXp+GwcYrz3hnyppOdwRgnmJ+nNsvbvcB8cr4n/6vlINHuPTyYwN2ZXe KWGw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:feedback-id:references :in-reply-to:message-id:subject:cc:from:to:date:dkim-signature; bh=oSUfZi1cGfdeB2IvoV8ev5peOvcG5sVLj13qHsWtvCc=; fh=Eq0QL0seKttJjxpSqDCwl1MLpMrKezUXw/E4rGsfgDY=; b=PvnHLUVwiF40zU8OyKympRPN5TAZEPkb9xB8ggsLMBWKZzIoF/dheU5kNAVfi6u1yq e8oX925YvkennsQJTXnibNUgNes/1xAePoi/D6aqsPoqhpk+vwfSE4JCoaMJMZIuorwR M20/Yv4r414aSZb9FV1RIAZPyNVKqJHEYP7Yoh6hNq543lfjqFIheEa9o0M0d8YqT17Q SSrLDXXVwpb8iqXraT/MLHXOm3w+NS2RFDXRnZYSJ3UxC24olvPrE7QlZWtjl6DGcssB Z/21zcTHkAtpS2xaQva3HsPONUirci/nIHD7V+mFTuWA4fa4RuwJoc/FzuMX4QIxgK7s 4Xkg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@proton.me header.s=protonmail header.b=UfYoVems; arc=pass (i=1 spf=pass spfdomain=proton.me dkim=pass dkdomain=proton.me dmarc=pass fromdomain=proton.me); spf=pass (google.com: domain of linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id c22-20020aa78816000000b006e537cc7299si1394874pfo.256.2024.02.29.06.48.34 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 06:48:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@proton.me header.s=protonmail header.b=UfYoVems; arc=pass (i=1 spf=pass spfdomain=proton.me dkim=pass dkdomain=proton.me dmarc=pass fromdomain=proton.me); spf=pass (google.com: domain of linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-86865-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 92E4A285DA3 for <ouuuleilei@gmail.com>; Thu, 29 Feb 2024 14:48:34 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 20947137767; Thu, 29 Feb 2024 14:48:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b="UfYoVems" Received: from mail-4319.protonmail.ch (mail-4319.protonmail.ch [185.70.43.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4962A41A80 for <linux-kernel@vger.kernel.org>; Thu, 29 Feb 2024 14:48:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.43.19 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709218099; cv=none; b=dXjC10M5S8XjlTo0YaTv+mGIpeadRdqj0Jw08oZK7cX+q4opK0ilGcUWLlCIbO0hotLmRTW0kn7knVT3EpxrZuWJztfQ1pNspy9SgaMP/cRfKBSPDBqTocHY/+8Ta8SuTidNJeZzFA6gk0kRZnCIfj94iKe51sXmrW2tM9Y+7ww= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709218099; c=relaxed/simple; bh=WGL9qM33Nk7fRGwrV2mf7JvJ3jNhdkTxb/e39Lrf8ZY=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=h+3oD74aVz/4sK9k/D94zC9xGhl8E6/J0PrQRFgJd75qTlHFe/QXl+A1VMudGMmUM9ih7ATlJcd57SBhyk0rMfPnNz3Jek3x/HSm5SNWJnUUasFiUeR+IVzn+5GZSH9Fqo5/vcFFqfwSJ7t2Qmu7Z0lFCv2kUEeGw+pg/MWChZk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me; spf=pass smtp.mailfrom=proton.me; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b=UfYoVems; arc=none smtp.client-ip=185.70.43.19 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=proton.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1709218090; x=1709477290; bh=oSUfZi1cGfdeB2IvoV8ev5peOvcG5sVLj13qHsWtvCc=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=UfYoVemsMrQXXTx6rDIs2sKrXk0meAWKVEgf/yHRQx7M1rDUxGzvpEqxfNE2Y74XE t1TiBTeQl3lF75ZupVeL9itL2H2iEebx1xVAQiDcbaCzdGvHy8ZL2nRS2BSGOSAQwg Eq4Yt0EWBcXzdh0fDzMj4fiBZbRz3tn+V26n/v8cb4Vw3fK8oC6XHqDd3TY1M4V+0P FmTlx3Kn4I8gJhPIqv0kftgY8YKYPDkPsbem+/HOIwDlw0Hwg1rs7p2Jit4YJ/OckL sEkVM19++zYWIL7lG9LC5G+gNutZklt4F/23hsa5cX6rrqjRbcHQ/OKhdiWi+dG4hi IxQpaaVxQFJ0w== Date: Thu, 29 Feb 2024 14:47:59 +0000 To: o-takashi@sakamocchi.jp From: Edmund Raile <edmund.raile@proton.me> Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Edmund Raile <edmund.raile@proton.me> Subject: [PATCH v2] firewire: ohci: prevent leak of left-over IRQ on unbind Message-ID: <20240229144723.13047-2-edmund.raile@proton.me> In-Reply-To: <20240229101236.8074-1-edmund.raile@proton.me> References: <20240229101236.8074-1-edmund.raile@proton.me> Feedback-ID: 45198251:user:proton Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1792245094095280030 X-GMAIL-MSGID: 1792245094095280030 |
Series |
[v2] firewire: ohci: prevent leak of left-over IRQ on unbind
|
|
Commit Message
Edmund Raile
Feb. 29, 2024, 2:47 p.m. UTC
Commit 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ")
also removed the call to free_irq() in pci_remove(), leading to a
leftover irq of devm_request_irq() at pci_disable_msi() in pci_remove()
when unbinding the driver from the device
remove_proc_entry: removing non-empty directory 'irq/136', leaking at
least 'firewire_ohci'
Call Trace:
? remove_proc_entry+0x19c/0x1c0
? __warn+0x81/0x130
? remove_proc_entry+0x19c/0x1c0
? report_bug+0x171/0x1a0
? console_unlock+0x78/0x120
? handle_bug+0x3c/0x80
? exc_invalid_op+0x17/0x70
? asm_exc_invalid_op+0x1a/0x20
? remove_proc_entry+0x19c/0x1c0
unregister_irq_proc+0xf4/0x120
free_desc+0x3d/0xe0
? kfree+0x29f/0x2f0
irq_free_descs+0x47/0x70
msi_domain_free_locked.part.0+0x19d/0x1d0
msi_domain_free_irqs_all_locked+0x81/0xc0
pci_free_msi_irqs+0x12/0x40
pci_disable_msi+0x4c/0x60
pci_remove+0x9d/0xc0 [firewire_ohci
01b483699bebf9cb07a3d69df0aa2bee71db1b26]
pci_device_remove+0x37/0xa0
device_release_driver_internal+0x19f/0x200
unbind_store+0xa1/0xb0
remove irq with devm_free_irq() before pci_disable_msi()
also remove it in fail_msi: of pci_probe() as this would lead to
an identical leak
Fixes: 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ")
Signed-off-by: Edmund Raile <edmund.raile@proton.me>
---
Using FW643 with vfio-pci required unbinding from firewire_ohci,
doing so currently produces a memory leak due to a leftover irq
which this patch removes.
The irq can be observed while the driver is loaded and bound:
find /proc/irq -type d -name "firewire_ohci"
Is it a good idea to submit a patch to devm_request_irq() in
include/linux/interrupt.h to add the function comment
/*
* counterpart: devm_free_irq()
*/
so LSPs show that hint?
v2 change: corrected patch title
drivers/firewire/ohci.c | 2 ++
1 file changed, 2 insertions(+)
Comments
Hi, Thanks for your taking the issue, and sending the patch. But I have a concern about the change, since the allocated IRQ should be released as the part of managed device resource[1]: (include/linux/interrupt.h) devm_request_irq() (kernel/irq/devres.c) ->devm_request_threaded_irq() ->devres_alloc(devm_irq_release) (kernel/irq/devres.c) devrm_irq_release() ->free_irq() In my opinion, the devres mechanism releases the allocated memory when releasing the data of associated device structure. In our case, it is the data of pci_dev structure (precisely, the data of device structure embedded in it). In the call trace of your commit comment, the release should be done in: (drivers/base/dd.c) device_release_driver_internal() ->__device_release_driver() ->device_unbind_cleanup() (drivers/base/devres.c) ->devres_release_all(dev); ->release_nodes() (kernel/irq/devres.c) ->free_irq() However, you encounter the leak actually. I think we have another cause for the leak, but never figured it out. Anyway, I'll further investigate the issue. [1] https://docs.kernel.org/driver-api/driver-model/devres.html On Thu, Feb 29, 2024 at 02:47:59PM +0000, Edmund Raile wrote: > > Commit 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ") > also removed the call to free_irq() in pci_remove(), leading to a > leftover irq of devm_request_irq() at pci_disable_msi() in pci_remove() > when unbinding the driver from the device > > remove_proc_entry: removing non-empty directory 'irq/136', leaking at > least 'firewire_ohci' > Call Trace: > ? remove_proc_entry+0x19c/0x1c0 > ? __warn+0x81/0x130 > ? remove_proc_entry+0x19c/0x1c0 > ? report_bug+0x171/0x1a0 > ? console_unlock+0x78/0x120 > ? handle_bug+0x3c/0x80 > ? exc_invalid_op+0x17/0x70 > ? asm_exc_invalid_op+0x1a/0x20 > ? remove_proc_entry+0x19c/0x1c0 > unregister_irq_proc+0xf4/0x120 > free_desc+0x3d/0xe0 > ? kfree+0x29f/0x2f0 > irq_free_descs+0x47/0x70 > msi_domain_free_locked.part.0+0x19d/0x1d0 > msi_domain_free_irqs_all_locked+0x81/0xc0 > pci_free_msi_irqs+0x12/0x40 > pci_disable_msi+0x4c/0x60 > pci_remove+0x9d/0xc0 [firewire_ohci > 01b483699bebf9cb07a3d69df0aa2bee71db1b26] > pci_device_remove+0x37/0xa0 > device_release_driver_internal+0x19f/0x200 > unbind_store+0xa1/0xb0 > > remove irq with devm_free_irq() before pci_disable_msi() > also remove it in fail_msi: of pci_probe() as this would lead to > an identical leak > > Fixes: 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ") > > Signed-off-by: Edmund Raile <edmund.raile@proton.me> > > --- > > Using FW643 with vfio-pci required unbinding from firewire_ohci, > doing so currently produces a memory leak due to a leftover irq > which this patch removes. > > The irq can be observed while the driver is loaded and bound: > find /proc/irq -type d -name "firewire_ohci" > > Is it a good idea to submit a patch to devm_request_irq() in > include/linux/interrupt.h to add the function comment > /* > * counterpart: devm_free_irq() > */ > so LSPs show that hint? > > v2 change: corrected patch title > > drivers/firewire/ohci.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c > index 9db9290c3269..7bc71f4be64a 100644 > --- a/drivers/firewire/ohci.c > +++ b/drivers/firewire/ohci.c > @@ -3773,6 +3773,7 @@ static int pci_probe(struct pci_dev *dev, > return 0; > > fail_msi: > + devm_free_irq(&dev->dev, dev->irq, ohci); > pci_disable_msi(dev); > > return err; > @@ -3800,6 +3801,7 @@ static void pci_remove(struct pci_dev *dev) > > software_reset(ohci); > > + devm_free_irq(&dev->dev, dev->irq, ohci); > pci_disable_msi(dev); > > dev_notice(&dev->dev, "removing fw-ohci device\n"); > -- > 2.43.0 Thanks Takashi Sakamoto
> In my opinion, the devres mechanism releases the allocated memory when > releasing the data of associated device structure. > device_release_driver_internal() > ->__device_release_driver() > ->device_unbind_cleanup() > (drivers/base/devres.c) > ->devres_release_all(dev); > ->release_nodes() > (kernel/irq/devres.c) > ->free_irq() Looking at __device_release_driver() in drivers/base/dd.c, device_remove() gets called, leading to dev->bus->remove(dev), which likely calls our good old friend from the call trace: pci_device_remove(). > > Call Trace: > > ? remove_proc_entry+0x19c/0x1c0 > > ? __warn+0x81/0x130 > > ? remove_proc_entry+0x19c/0x1c0 > > ? report_bug+0x171/0x1a0 > > ? console_unlock+0x78/0x120 > > ? handle_bug+0x3c/0x80 > > ? exc_invalid_op+0x17/0x70 > > ? asm_exc_invalid_op+0x1a/0x20 > > ? remove_proc_entry+0x19c/0x1c0 > > unregister_irq_proc+0xf4/0x120 > > free_desc+0x3d/0xe0 > > ? kfree+0x29f/0x2f0 > > irq_free_descs+0x47/0x70 > > msi_domain_free_locked.part.0+0x19d/0x1d0 > > msi_domain_free_irqs_all_locked+0x81/0xc0 > > pci_free_msi_irqs+0x12/0x40 > > pci_disable_msi+0x4c/0x60 > > pci_remove+0x9d/0xc0 [firewire_ohci > > 01b483699bebf9cb07a3d69df0aa2bee71db1b26] > > pci_device_remove+0x37/0xa0 > > device_release_driver_internal+0x19f/0x200 > > unbind_store+0xa1/0xb0 Then in ohci.c's pci_remove(), we kill the MSIs, which leads to the removal of the IRQ, etc. Back in __device_release_driver(), after device_remove(), device_unbind_cleanup() is called, leading to free_irq(), but too late. I think the order of these calls may be our issue but I doubt it has been done like this without good reason. That code is 8 years old, someone would have noticed if it had an error. I could be entirely wrong but the function description in /kernel/irq/devres.c tells me that function is meant to be used: > Except for the extra @dev argument, this function takes the > same arguments and performs the same function as free_irq(). > This function instead of free_irq() should be used to manually > free IRQs allocated with devm_request_irq(). And while devm_request_irq() has no function description of its own, its sister devm_request_threaded_irq() mentions this: > IRQs requested with this function will be > automatically freed on driver detach. > > If an IRQ allocated with this function needs to be freed > separately, devm_free_irq() must be used. Should we pull in the maintainers of dd.c for their opinion? Thank you very much for all the very hard work you do Sakamoto-Sensei!
Hi, (C.C.ed to the list of PCI SUBSYSTEM.) On Sat, Mar 02, 2024 at 04:52:06PM +0000, Edmund Raile wrote: > > In my opinion, the devres mechanism releases the allocated memory when > > releasing the data of associated device structure. > > device_release_driver_internal() > > ->__device_release_driver() > > ->device_unbind_cleanup() > > (drivers/base/devres.c) > > ->devres_release_all(dev); > > ->release_nodes() > > (kernel/irq/devres.c) > > ->free_irq() > > Looking at __device_release_driver() in drivers/base/dd.c, > device_remove() gets called, leading to dev->bus->remove(dev), > which likely calls our good old friend from the call trace: > pci_device_remove(). > > > > Call Trace: > > > ? remove_proc_entry+0x19c/0x1c0 > > > ? __warn+0x81/0x130 > > > ? remove_proc_entry+0x19c/0x1c0 > > > ? report_bug+0x171/0x1a0 > > > ? console_unlock+0x78/0x120 > > > ? handle_bug+0x3c/0x80 > > > ? exc_invalid_op+0x17/0x70 > > > ? asm_exc_invalid_op+0x1a/0x20 > > > ? remove_proc_entry+0x19c/0x1c0 > > > unregister_irq_proc+0xf4/0x120 > > > free_desc+0x3d/0xe0 > > > ? kfree+0x29f/0x2f0 > > > irq_free_descs+0x47/0x70 > > > msi_domain_free_locked.part.0+0x19d/0x1d0 > > > msi_domain_free_irqs_all_locked+0x81/0xc0 > > > pci_free_msi_irqs+0x12/0x40 > > > pci_disable_msi+0x4c/0x60 > > > pci_remove+0x9d/0xc0 [firewire_ohci > > > 01b483699bebf9cb07a3d69df0aa2bee71db1b26] > > > pci_device_remove+0x37/0xa0 > > > device_release_driver_internal+0x19f/0x200 > > > unbind_store+0xa1/0xb0 > > Then in ohci.c's pci_remove(), we kill the MSIs, which leads to > the removal of the IRQ, etc. > Back in __device_release_driver(), after device_remove(), > device_unbind_cleanup() is called, leading to free_irq(), but too late. > > I think the order of these calls may be our issue but I doubt it > has been done like this without good reason. > That code is 8 years old, someone would have noticed if it had an error. Now I got the point. Before optimizing to device managing resource, the 1394 OHCI driver called `free_irq()` then `pci_disable_msi()` in the remove() callback. So the issue did not occur. At present, the order is reversed, as you find. To be honestly, I have little knowledge about current implementation of PCIe MSI operation and the current best-practice in Linux PCI subsystem. I've just replaced the old implementation of the driver with the relevant APIs, so I need to consult someone about the issue. > I could be entirely wrong but the function description in > /kernel/irq/devres.c tells me that function is meant to be used: > > > Except for the extra @dev argument, this function takes the > > same arguments and performs the same function as free_irq(). > > This function instead of free_irq() should be used to manually > > free IRQs allocated with devm_request_irq(). > > And while devm_request_irq() has no function description of its own, its > sister devm_request_threaded_irq() mentions this: > > > IRQs requested with this function will be > > automatically freed on driver detach. > > > > If an IRQ allocated with this function needs to be freed > > separately, devm_free_irq() must be used. > > Should we pull in the maintainers of dd.c for their opinion? > > Thank you very much for all the very hard work you do Sakamoto-Sensei! Indeed. If the current implementation of PCIe MSI requires the call of `free_irq()` (or something) before calling `pci_disable_msi()`, it should be documented. But we can also see the `pci_disable_msi()` is legacy API in PCIe MSI implementation[1]. I guess that the extra care of order to call these two functions would be useless nowadays by some enhancement. [1] https://docs.kernel.org/PCI/msi-howto.html#legacy-apis Thanks Takashi Sakamoto
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 9db9290c3269..7bc71f4be64a 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -3773,6 +3773,7 @@ static int pci_probe(struct pci_dev *dev, return 0; fail_msi: + devm_free_irq(&dev->dev, dev->irq, ohci); pci_disable_msi(dev); return err; @@ -3800,6 +3801,7 @@ static void pci_remove(struct pci_dev *dev) software_reset(ohci); + devm_free_irq(&dev->dev, dev->irq, ohci); pci_disable_msi(dev); dev_notice(&dev->dev, "removing fw-ohci device\n");