| Message ID | 20220621121147.3971001-1-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: endpoint: Fix WARN() when an endpoint driver is removed | expand |
On Tue, Jun 21, 2022 at 09:11:47PM +0900, Yoshihiro Shimoda wrote: > Add pci_epc_nop_release() for epc->dev.release. Otherwise, > WARN() happened when a PCIe endpoint driver is removed. > > Device 'e65d0000.pcie-ep' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst. > WARNING: CPU: 0 PID: 139 at drivers/base/core.c:2232 device_release+0x78/0x8c > Hmm... So the core is correctly throwing the warning here but just using a NOP release function is not the right solution IMO. Currently, pci_epc_destroy() is handling all resource release including freeing the memory for epc. But I think the "kfree(epc)" should be moved to this release callback. Reason is quite obvious. Until the release() callback gets called, there would be a "epc" device instance floating around and we should not free it until then. Thanks, Mani > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > drivers/pci/endpoint/pci-epc-core.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > index 3bc9273d0a08..cb533821b072 100644 > --- a/drivers/pci/endpoint/pci-epc-core.c > +++ b/drivers/pci/endpoint/pci-epc-core.c > @@ -746,6 +746,10 @@ void devm_pci_epc_destroy(struct device *dev, struct pci_epc *epc) > } > EXPORT_SYMBOL_GPL(devm_pci_epc_destroy); > > +static void pci_epc_nop_release(struct device *dev) > +{ > +} > + > /** > * __pci_epc_create() - create a new endpoint controller (EPC) device > * @dev: device that is creating the new EPC > @@ -779,6 +783,7 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops, > device_initialize(&epc->dev); > epc->dev.class = pci_epc_class; > epc->dev.parent = dev; > + epc->dev.release = pci_epc_nop_release; > epc->ops = ops; > > ret = dev_set_name(&epc->dev, "%s", dev_name(dev)); > -- > 2.25.1 >
Hi Manivannan, > From: Manivannan Sadhasivam, Sent: Tuesday, June 21, 2022 11:21 PM > > On Tue, Jun 21, 2022 at 09:11:47PM +0900, Yoshihiro Shimoda wrote: > > Add pci_epc_nop_release() for epc->dev.release. Otherwise, > > WARN() happened when a PCIe endpoint driver is removed. > > > > Device 'e65d0000.pcie-ep' does not have a release() function, it is broken and must be fixed. See > Documentation/core-api/kobject.rst. > > WARNING: CPU: 0 PID: 139 at drivers/base/core.c:2232 device_release+0x78/0x8c > > > > Hmm... So the core is correctly throwing the warning here but just using a NOP > release function is not the right solution IMO. > > Currently, pci_epc_destroy() is handling all resource release including freeing > the memory for epc. But I think the "kfree(epc)" should be moved to this > release callback. Reason is quite obvious. Until the release() callback gets > called, there would be a "epc" device instance floating around and we should not > free it until then. I understood it. Especially, we enable CONFIG_DEBUG_KOBJECT_RELEASE=y, the release() callback will be called after pci_epc_destroy() exited. So, I'll move "kfree(epc)" to the release function. Best regards, Yoshihiro Shimoda > Thanks, > Mani > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > drivers/pci/endpoint/pci-epc-core.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > > index 3bc9273d0a08..cb533821b072 100644 > > --- a/drivers/pci/endpoint/pci-epc-core.c > > +++ b/drivers/pci/endpoint/pci-epc-core.c > > @@ -746,6 +746,10 @@ void devm_pci_epc_destroy(struct device *dev, struct pci_epc *epc) > > } > > EXPORT_SYMBOL_GPL(devm_pci_epc_destroy); > > > > +static void pci_epc_nop_release(struct device *dev) > > +{ > > +} > > + > > /** > > * __pci_epc_create() - create a new endpoint controller (EPC) device > > * @dev: device that is creating the new EPC > > @@ -779,6 +783,7 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops, > > device_initialize(&epc->dev); > > epc->dev.class = pci_epc_class; > > epc->dev.parent = dev; > > + epc->dev.release = pci_epc_nop_release; > > epc->ops = ops; > > > > ret = dev_set_name(&epc->dev, "%s", dev_name(dev)); > > -- > > 2.25.1 > > > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c index 3bc9273d0a08..cb533821b072 100644 --- a/drivers/pci/endpoint/pci-epc-core.c +++ b/drivers/pci/endpoint/pci-epc-core.c @@ -746,6 +746,10 @@ void devm_pci_epc_destroy(struct device *dev, struct pci_epc *epc) } EXPORT_SYMBOL_GPL(devm_pci_epc_destroy); +static void pci_epc_nop_release(struct device *dev) +{ +} + /** * __pci_epc_create() - create a new endpoint controller (EPC) device * @dev: device that is creating the new EPC @@ -779,6 +783,7 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops, device_initialize(&epc->dev); epc->dev.class = pci_epc_class; epc->dev.parent = dev; + epc->dev.release = pci_epc_nop_release; epc->ops = ops; ret = dev_set_name(&epc->dev, "%s", dev_name(dev));
Add pci_epc_nop_release() for epc->dev.release. Otherwise, WARN() happened when a PCIe endpoint driver is removed. Device 'e65d0000.pcie-ep' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst. WARNING: CPU: 0 PID: 139 at drivers/base/core.c:2232 device_release+0x78/0x8c Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- drivers/pci/endpoint/pci-epc-core.c | 5 +++++ 1 file changed, 5 insertions(+)