@@ -75,18 +75,20 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)structpci_dev*virtfn;structresource*res;structpci_sriov*iov=dev->sriov;+structpci_bus*bus;-virtfn=alloc_pci_dev();+virtfn=pci_alloc_dev(NULL);if(!virtfn)return-ENOMEM;mutex_lock(&iov->dev->sriov->lock);-virtfn->bus=virtfn_add_bus(dev->bus,virtfn_bus(dev,id));-if(!virtfn->bus){+bus=virtfn_add_bus(dev->bus,virtfn_bus(dev,id));+if(!bus){kfree(virtfn);mutex_unlock(&iov->dev->sriov->lock);return-ENOMEM;}+virtfn->bus=pci_bus_get(bus);virtfn->devfn=virtfn_devfn(dev,id);virtfn->vendor=dev->vendor;pci_read_config_word(dev,iov->pos+PCI_SRIOV_VF_DID,&virtfn->device);
From: Yinghai Lu <yinghai@kernel.org> Date: 2013-05-14 15:10:37
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted hunk
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs to
release OF related resources.
I will update it in next version.
From: Liu Jiang <hidden> Date: 2013-05-14 16:57:36
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs to
release OF related resources.
I will update it in next version.
From: Yinghai Lu <yinghai@kernel.org> Date: 2013-05-14 18:52:55
On Tue, May 14, 2013 at 9:57 AM, Liu Jiang [off-list ref] wrote:
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs to
release OF related resources.
I will update it in next version.
From: Liu Jiang <hidden> Date: 2013-05-15 14:39:28
On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote:
On Tue, May 14, 2013 at 9:57 AM, Liu Jiang [off-list ref] wrote:
quoted
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs to
release OF related resources.
I will update it in next version.
no, should move pci_set_of_node calling into pci_setup_device.
Yinghai
I'm not sure whether we should call pci_set_of_node() for SR-IOV devices
too,
any suggestions here?
or just move down pci_set_of_node after pci_setup_device?
anyway that is another bug.
Yinghai
I'm not familiar with the OF logic and can't make sure whether
pci_setup_device()
has dependency on dev->of_node. Feel it's more safe to call
pci_release_of_node()
on failing path instead of tuning call-site of pci_set_of_node().
From: Yinghai Lu <yinghai@kernel.org> Date: 2013-05-15 14:43:07
On Wed, May 15, 2013 at 7:39 AM, Liu Jiang [off-list ref] wrote:
On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 9:57 AM, Liu Jiang [off-list ref] wrote:
quoted
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs
to
release OF related resources.
I will update it in next version.
no, should move pci_set_of_node calling into pci_setup_device.
Yinghai
I'm not sure whether we should call pci_set_of_node() for SR-IOV devices
too,
any suggestions here?
or just move down pci_set_of_node after pci_setup_device?
anyway that is another bug.
I'm not familiar with the OF logic and can't make sure whether
pci_setup_device()
has dependency on dev->of_node. Feel it's more safe to call
pci_release_of_node()
on failing path instead of tuning call-site of pci_set_of_node().
that is another bug, let of guy handle it.
Yinghai
From: Liu Jiang <hidden> Date: 2013-05-15 14:46:21
On Wed 15 May 2013 10:43:02 PM CST, Yinghai Lu wrote:
On Wed, May 15, 2013 at 7:39 AM, Liu Jiang [off-list ref] wrote:
quoted
On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 9:57 AM, Liu Jiang [off-list ref] wrote:
quoted
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote:
quoted
On Tue, May 14, 2013 at 7:59 AM, Liu Jiang [off-list ref] wrote:
quoted
On 05/14/2013 04:26 PM, Gu Zheng wrote:
I suggest to use pci_release_dev() instead because it also needs
to
release OF related resources.
I will update it in next version.
no, should move pci_set_of_node calling into pci_setup_device.
Yinghai
I'm not sure whether we should call pci_set_of_node() for SR-IOV devices
too,
any suggestions here?
or just move down pci_set_of_node after pci_setup_device?
anyway that is another bug.
quoted
I'm not familiar with the OF logic and can't make sure whether
pci_setup_device()
has dependency on dev->of_node. Feel it's more safe to call
pci_release_of_node()
on failing path instead of tuning call-site of pci_set_of_node().
that is another bug, let of guy handle it.
Yinghai
Hi Yinghai,
I don't know any OF exports, could you please help to CC
some OF experts?
Thanks,
Gerry
From: Yinghai Lu <yinghai@kernel.org> Date: 2013-05-15 14:58:48
On Wed, May 15, 2013 at 7:46 AM, Liu Jiang [off-list ref] wrote:
On Wed 15 May 2013 10:43:02 PM CST, Yinghai Lu wrote:
quotedquoted
that is another bug, let of guy handle it.
Yinghai
Hi Yinghai,
I don't know any OF exports, could you please help to CC
some OF experts?
powerpc and sparc are using that.
Ben,
in drivers/pci/probe.c::pci_scan_device() there is
pci_set_of_node(dev);
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
so if pci_setup_device fails, there is one dev reference is not release.
please check you can just move down pci_set_of_node down after that
failing path, like
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
pci_set_of_node(dev);
Yinghai
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2013-05-15 21:33:03
On Wed, 2013-05-15 at 07:58 -0700, Yinghai Lu wrote:
Ben,
in drivers/pci/probe.c::pci_scan_device() there is
pci_set_of_node(dev);
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
so if pci_setup_device fails, there is one dev reference is not release.
please check you can just move down pci_set_of_node down after that
failing path, like
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
pci_set_of_node(dev);
No, we want the OF node set when we run the quirks, we intentionally do
that early, the right thing to do is to to call pci_release_of_node()
in the error path (it's safe to call even if the node is NULL).
Cheers,
Ben.
From: Yinghai Lu <yinghai@kernel.org> Date: 2013-05-15 21:52:26
On Wed, May 15, 2013 at 2:32 PM, Benjamin Herrenschmidt
[off-list ref] wrote:
On Wed, 2013-05-15 at 07:58 -0700, Yinghai Lu wrote:
quoted
Ben,
in drivers/pci/probe.c::pci_scan_device() there is
pci_set_of_node(dev);
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
so if pci_setup_device fails, there is one dev reference is not release.
please check you can just move down pci_set_of_node down after that
failing path, like
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
pci_set_of_node(dev);
No, we want the OF node set when we run the quirks, we intentionally do
that early, the right thing to do is to to call pci_release_of_node()
in the error path (it's safe to call even if the node is NULL).
Good.
We have two options.
1. can you please submit one complete patch, and get it merged into v3.10.
2. put it together with pci_alloc_dev patchset towards to v3.11?
Thanks
Yinghai
From: Liu Jiang <hidden> Date: 2013-05-15 23:47:12
On Thu 16 May 2013 05:29:31 AM CST, Benjamin Herrenschmidt wrote:
On Wed, 2013-05-15 at 22:46 +0800, Liu Jiang wrote:
quoted
I don't know any OF exports, could you please help to CC
some OF experts?
I wrote that code I think. Sorry, I've missed the beginning of the
thread, what is the problem ?
Cheers,
Ben.
Hi,
Just found a little memory leak issue that we should call
pci_release_of_node()
on error recovery path in function pci_scan_device().
pci_set_of_node(dev);
if (pci_setup_device(dev)) {
kfree(dev);
return NULL;
}
Regards!
Gerry