Patchwork [v8,06/22] PCI: split registration of PCI bus devices into two stages

login
register
mail settings
Submitter Yinghai Lu
Date Jan. 11, 2013, 10:40 p.m.
Message ID <1357944049-29620-7-git-send-email-yinghai@kernel.org>
Download mbox | patch
Permalink /patch/211440/
State Superseded
Headers show

Comments

Yinghai Lu - Jan. 11, 2013, 10:40 p.m.
From: Jiang Liu <jiang.liu@huawei.com>

When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
the notification handler can't hold reference count to the new PCI bus
because the device object for the new bus (pci_dev->subordinate->dev)
hasn't been initialized yet.

Split the registration of PCI bus device into two stages as below,
so that the event handler could hold reference count to the new PCI bus
when handling BUS_NOTIFY_ADD_DEVICE event.

1) device_initialize(&pci_dev->dev)
2) device_initialize(&pci_dev->subordinate->dev)
3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
4) device_add(&pci_dev->dev)
5) device_add(&pci_dev->subordinate->dev)

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/bus.c    |    2 +-
 drivers/pci/probe.c  |    4 +++-
 drivers/pci/remove.c |   10 +++++-----
 3 files changed, 9 insertions(+), 7 deletions(-)
Rafael J. Wysocki - Jan. 12, 2013, 10:34 p.m.
On Friday, January 11, 2013 02:40:33 PM Yinghai Lu wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
> 
> When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
> the notification handler can't hold reference count to the new PCI bus
> because the device object for the new bus (pci_dev->subordinate->dev)
> hasn't been initialized yet.

It doesn't look like BUS_NOTIFY_ADD_DEVICE is used by the PCI bus type
in the mainline.

Is it in linux-next?

If not, then I don't see why we need this patch.

Thanks,
Rafael


> Split the registration of PCI bus device into two stages as below,
> so that the event handler could hold reference count to the new PCI bus
> when handling BUS_NOTIFY_ADD_DEVICE event.
> 
> 1) device_initialize(&pci_dev->dev)
> 2) device_initialize(&pci_dev->subordinate->dev)
> 3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
> 4) device_add(&pci_dev->dev)
> 5) device_add(&pci_dev->subordinate->dev)
> 
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/bus.c    |    2 +-
>  drivers/pci/probe.c  |    4 +++-
>  drivers/pci/remove.c |   10 +++++-----
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 847f3ca..5f9c728 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -200,7 +200,7 @@ int pci_bus_add_child(struct pci_bus *bus)
>  	if (bus->bridge)
>  		bus->dev.parent = bus->bridge;
>  
> -	retval = device_register(&bus->dev);
> +	retval = device_add(&bus->dev);
>  	if (retval)
>  		return retval;
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 48b35e1..dc4fde3 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -642,6 +642,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  	 */
>  	child->dev.class = &pcibus_class;
>  	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
> +	device_initialize(&child->dev);
>  
>  	/*
>  	 * Set up the primary, secondary and subordinate
> @@ -1678,7 +1679,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	b->dev.class = &pcibus_class;
>  	b->dev.parent = b->bridge;
>  	dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
> -	error = device_register(&b->dev);
> +	device_initialize(&b->dev);
> +	error = device_add(&b->dev);
>  	if (error)
>  		goto class_dev_reg_err;
>  
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index fc38c48..a1fdd0f 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
>  	list_del(&bus->node);
>  	pci_bus_release_busn_res(bus);
>  	up_write(&pci_bus_sem);
> -	if (!bus->is_added)
> -		return;
> -
> -	pci_remove_legacy_files(bus);
> -	device_unregister(&bus->dev);
> +	if (bus->is_added) {
> +		pci_remove_legacy_files(bus);
> +		device_del(&bus->dev);
> +	}
> +	put_device(&bus->dev);
>  }
>  EXPORT_SYMBOL(pci_remove_bus);
>  
>
Jiang Liu - Jan. 13, 2013, 3:25 p.m.
On 01/13/2013 06:34 AM, Rafael J. Wysocki wrote:
> On Friday, January 11, 2013 02:40:33 PM Yinghai Lu wrote:
>> From: Jiang Liu <jiang.liu@huawei.com>
>>
>> When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
>> the notification handler can't hold reference count to the new PCI bus
>> because the device object for the new bus (pci_dev->subordinate->dev)
>> hasn't been initialized yet.
> 
> It doesn't look like BUS_NOTIFY_ADD_DEVICE is used by the PCI bus type
> in the mainline.
> 
> Is it in linux-next?
> 
> If not, then I don't see why we need this patch.
Hi Rafael,
	Originally it's a preparation patch for pci_slot/acpiphp/pci_bind
related fixups. With recent work from you to remove bind/unbind from struct
acpi_bus_type, we have no dependency on this patch anymore.
Regards!
Gerry

> 
> Thanks,
> Rafael
> 
> 
>> Split the registration of PCI bus device into two stages as below,
>> so that the event handler could hold reference count to the new PCI bus
>> when handling BUS_NOTIFY_ADD_DEVICE event.
>>
>> 1) device_initialize(&pci_dev->dev)
>> 2) device_initialize(&pci_dev->subordinate->dev)
>> 3) notify BUS_NOTIFY_ADD_DEVICE event for pci_dev
>> 4) device_add(&pci_dev->dev)
>> 5) device_add(&pci_dev->subordinate->dev)
>>
>> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/bus.c    |    2 +-
>>  drivers/pci/probe.c  |    4 +++-
>>  drivers/pci/remove.c |   10 +++++-----
>>  3 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
>> index 847f3ca..5f9c728 100644
>> --- a/drivers/pci/bus.c
>> +++ b/drivers/pci/bus.c
>> @@ -200,7 +200,7 @@ int pci_bus_add_child(struct pci_bus *bus)
>>  	if (bus->bridge)
>>  		bus->dev.parent = bus->bridge;
>>  
>> -	retval = device_register(&bus->dev);
>> +	retval = device_add(&bus->dev);
>>  	if (retval)
>>  		return retval;
>>  
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 48b35e1..dc4fde3 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -642,6 +642,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>>  	 */
>>  	child->dev.class = &pcibus_class;
>>  	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
>> +	device_initialize(&child->dev);
>>  
>>  	/*
>>  	 * Set up the primary, secondary and subordinate
>> @@ -1678,7 +1679,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>  	b->dev.class = &pcibus_class;
>>  	b->dev.parent = b->bridge;
>>  	dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
>> -	error = device_register(&b->dev);
>> +	device_initialize(&b->dev);
>> +	error = device_add(&b->dev);
>>  	if (error)
>>  		goto class_dev_reg_err;
>>  
>> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
>> index fc38c48..a1fdd0f 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -48,11 +48,11 @@ void pci_remove_bus(struct pci_bus *bus)
>>  	list_del(&bus->node);
>>  	pci_bus_release_busn_res(bus);
>>  	up_write(&pci_bus_sem);
>> -	if (!bus->is_added)
>> -		return;
>> -
>> -	pci_remove_legacy_files(bus);
>> -	device_unregister(&bus->dev);
>> +	if (bus->is_added) {
>> +		pci_remove_legacy_files(bus);
>> +		device_del(&bus->dev);
>> +	}
>> +	put_device(&bus->dev);
>>  }
>>  EXPORT_SYMBOL(pci_remove_bus);
>>  
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu - Jan. 15, 2013, 6:29 a.m.
On Sun, Jan 13, 2013 at 7:25 AM, Jiang Liu <liuj97@gmail.com> wrote:
> On 01/13/2013 06:34 AM, Rafael J. Wysocki wrote:
>> On Friday, January 11, 2013 02:40:33 PM Yinghai Lu wrote:
>>> From: Jiang Liu <jiang.liu@huawei.com>
>>>
>>> When handling BUS_NOTIFY_ADD_DEVICE event for a PCI bridge device,
>>> the notification handler can't hold reference count to the new PCI bus
>>> because the device object for the new bus (pci_dev->subordinate->dev)
>>> hasn't been initialized yet.
>>
>> It doesn't look like BUS_NOTIFY_ADD_DEVICE is used by the PCI bus type
>> in the mainline.
>>
>> Is it in linux-next?
>>
>> If not, then I don't see why we need this patch.
> Hi Rafael,
>         Originally it's a preparation patch for pci_slot/acpiphp/pci_bind
> related fixups. With recent work from you to remove bind/unbind from struct
> acpi_bus_type, we have no dependency on this patch anymore.

ok, will drop this one.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 847f3ca..5f9c728 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -200,7 +200,7 @@  int pci_bus_add_child(struct pci_bus *bus)
 	if (bus->bridge)
 		bus->dev.parent = bus->bridge;
 
-	retval = device_register(&bus->dev);
+	retval = device_add(&bus->dev);
 	if (retval)
 		return retval;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 48b35e1..dc4fde3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -642,6 +642,7 @@  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
 	 */
 	child->dev.class = &pcibus_class;
 	dev_set_name(&child->dev, "%04x:%02x", pci_domain_nr(child), busnr);
+	device_initialize(&child->dev);
 
 	/*
 	 * Set up the primary, secondary and subordinate
@@ -1678,7 +1679,8 @@  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	b->dev.class = &pcibus_class;
 	b->dev.parent = b->bridge;
 	dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
-	error = device_register(&b->dev);
+	device_initialize(&b->dev);
+	error = device_add(&b->dev);
 	if (error)
 		goto class_dev_reg_err;
 
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index fc38c48..a1fdd0f 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -48,11 +48,11 @@  void pci_remove_bus(struct pci_bus *bus)
 	list_del(&bus->node);
 	pci_bus_release_busn_res(bus);
 	up_write(&pci_bus_sem);
-	if (!bus->is_added)
-		return;
-
-	pci_remove_legacy_files(bus);
-	device_unregister(&bus->dev);
+	if (bus->is_added) {
+		pci_remove_legacy_files(bus);
+		device_del(&bus->dev);
+	}
+	put_device(&bus->dev);
 }
 EXPORT_SYMBOL(pci_remove_bus);