diff mbox

[net] PCI: fix the return value for the pci_find_pcie_root_port()

Message ID 1502936730-7368-1-git-send-email-dingtianhong@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ding Tianhong Aug. 17, 2017, 2:25 a.m. UTC
The pci_find_pcie_root_port() would return NULL if the given
dev is already a Root Port, it looks like unfriendly to the
PCIe Root Port device, Thierry and Bjorn suggest to let this
function return the given dev under this circumstances.

Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Miller Aug. 17, 2017, 2:42 a.m. UTC | #1
From: Ding Tianhong <dingtianhong@huawei.com>
Date: Thu, 17 Aug 2017 10:25:30 +0800

> The pci_find_pcie_root_port() would return NULL if the given
> dev is already a Root Port, it looks like unfriendly to the
> PCIe Root Port device, Thierry and Bjorn suggest to let this
> function return the given dev under this circumstances.
> 
> Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>

Bjorn, please review and ACK so Linus doesn't rip my head off again
:-)

Thanks.
Thierry Reding Aug. 17, 2017, 10:51 a.m. UTC | #2
On Thu, Aug 17, 2017 at 10:25:30AM +0800, Ding Tianhong wrote:
> The pci_find_pcie_root_port() would return NULL if the given
> dev is already a Root Port, it looks like unfriendly to the
> PCIe Root Port device, Thierry and Bjorn suggest to let this
> function return the given dev under this circumstances.
> 
> Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  drivers/pci/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7e2022f..352bb53 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -514,7 +514,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
>   */
>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>  {
> -	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> +	struct pci_dev *bridge, *highest_pcie_bridge = dev;
>  
>  	bridge = pci_upstream_bridge(dev);
>  	while (bridge && pci_is_pcie(bridge)) {

I think this should actually be this change on top of a revert of commit
0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI
device"). After the above change, the previous fix will have a redundant
check because highest_pcie_bridge will never be NULL.

Let me send out that version to clarify what I mean.

Thierry
Ding Tianhong Aug. 17, 2017, 12:40 p.m. UTC | #3
On 2017/8/17 18:51, Thierry Reding wrote:
> On Thu, Aug 17, 2017 at 10:25:30AM +0800, Ding Tianhong wrote:
>> The pci_find_pcie_root_port() would return NULL if the given
>> dev is already a Root Port, it looks like unfriendly to the
>> PCIe Root Port device, Thierry and Bjorn suggest to let this
>> function return the given dev under this circumstances.
>>
>> Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
>> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
>> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
>> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>>  drivers/pci/pci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 7e2022f..352bb53 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -514,7 +514,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
>>   */
>>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>>  {
>> -	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
>> +	struct pci_dev *bridge, *highest_pcie_bridge = dev;
>>  
>>  	bridge = pci_upstream_bridge(dev);
>>  	while (bridge && pci_is_pcie(bridge)) {
> 
> I think this should actually be this change on top of a revert of commit
> 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI
> device"). After the above change, the previous fix will have a redundant
> check because highest_pcie_bridge will never be NULL.
> 
> Let me send out that version to clarify what I mean.
> 

Hi Thierry:

The patch ("PCI: fix oops when try to find Root Port for a PCI device")
has been merge to the linus mainline tree before you found this deficiencies....

Regards
Tianhong

> Thierry
>
Thierry Reding Aug. 17, 2017, 1:30 p.m. UTC | #4
On Thu, Aug 17, 2017 at 08:40:16PM +0800, Ding Tianhong wrote:
> 
> 
> On 2017/8/17 18:51, Thierry Reding wrote:
> > On Thu, Aug 17, 2017 at 10:25:30AM +0800, Ding Tianhong wrote:
> >> The pci_find_pcie_root_port() would return NULL if the given
> >> dev is already a Root Port, it looks like unfriendly to the
> >> PCIe Root Port device, Thierry and Bjorn suggest to let this
> >> function return the given dev under this circumstances.
> >>
> >> Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
> >> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
> >> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> >> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> >> ---
> >>  drivers/pci/pci.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> >> index 7e2022f..352bb53 100644
> >> --- a/drivers/pci/pci.c
> >> +++ b/drivers/pci/pci.c
> >> @@ -514,7 +514,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
> >>   */
> >>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
> >>  {
> >> -	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> >> +	struct pci_dev *bridge, *highest_pcie_bridge = dev;
> >>  
> >>  	bridge = pci_upstream_bridge(dev);
> >>  	while (bridge && pci_is_pcie(bridge)) {
> > 
> > I think this should actually be this change on top of a revert of commit
> > 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI
> > device"). After the above change, the previous fix will have a redundant
> > check because highest_pcie_bridge will never be NULL.
> > 
> > Let me send out that version to clarify what I mean.
> > 
> 
> Hi Thierry:
> 
> The patch ("PCI: fix oops when try to find Root Port for a PCI device")
> has been merge to the linus mainline tree before you found this deficiencies....

I understand that. I'm just saying that there's no point keeping that
change around because it no longer makes sense after we initialize the
highest_pcie_bridge variable to dev.

Thierry
Ding Tianhong Aug. 17, 2017, 1:38 p.m. UTC | #5
On 2017/8/17 21:30, Thierry Reding wrote:
> On Thu, Aug 17, 2017 at 08:40:16PM +0800, Ding Tianhong wrote:
>>
>>
>> On 2017/8/17 18:51, Thierry Reding wrote:
>>> On Thu, Aug 17, 2017 at 10:25:30AM +0800, Ding Tianhong wrote:
>>>> The pci_find_pcie_root_port() would return NULL if the given
>>>> dev is already a Root Port, it looks like unfriendly to the
>>>> PCIe Root Port device, Thierry and Bjorn suggest to let this
>>>> function return the given dev under this circumstances.
>>>>
>>>> Fixes: 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI device")
>>>> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
>>>> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
>>>> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
>>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>>> ---
>>>>  drivers/pci/pci.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index 7e2022f..352bb53 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -514,7 +514,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
>>>>   */
>>>>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>>>>  {
>>>> -	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
>>>> +	struct pci_dev *bridge, *highest_pcie_bridge = dev;
>>>>  
>>>>  	bridge = pci_upstream_bridge(dev);
>>>>  	while (bridge && pci_is_pcie(bridge)) {
>>>
>>> I think this should actually be this change on top of a revert of commit
>>> 0e405232871d6 ("PCI: fix oops when try to find Root Port for a PCI
>>> device"). After the above change, the previous fix will have a redundant
>>> check because highest_pcie_bridge will never be NULL.
>>>
>>> Let me send out that version to clarify what I mean.
>>>
>>
>> Hi Thierry:
>>
>> The patch ("PCI: fix oops when try to find Root Port for a PCI device")
>> has been merge to the linus mainline tree before you found this deficiencies....
> 
> I understand that. I'm just saying that there's no point keeping that
> change around because it no longer makes sense after we initialize the
> highest_pcie_bridge variable to dev.
> 

Ok, NO problem.:)

> Thierry
>
diff mbox

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7e2022f..352bb53 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -514,7 +514,7 @@  struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
  */
 struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
 {
-	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
+	struct pci_dev *bridge, *highest_pcie_bridge = dev;
 
 	bridge = pci_upstream_bridge(dev);
 	while (bridge && pci_is_pcie(bridge)) {