It seems my previous e-mail was not sent properly. So resending it.

Thanks,
Kenji Kaneshige



--

[ message continues ]

Yes, I've changed pci_get_pci_slot() to acquire the pci_bus_sem
semaphore.

Thank you for pointing this out.

It will be fixed in v4 of this patch series, which I will send
out after I receive the rest of your review comments.

Thanks!

/ac

--

[ message continues ]

Alex-san,


I noticed that changing pci_get_pci_slot() to acquire the pci_bus_sem
might be not enough. If slot was created between pci_get_pci_slot() and
pci_create_slot() by another thread in the following code, something
wrong would happen I think.

        pci_slot = pci_get_pci_slot(bus, slot_nr);
        if (pci_slot) {
                if (pci_slot->hotplug) {
                        result = -EBUSY;
                        goto err;
                }

                if (strcmp(kobject_name(&pci_slot->kobj), name))
                        if ((result = pci_rename_slot(pci_slot, name)))
                                goto err;
        } else {
                pci_slot = pci_create_slot(bus, slot_nr, name);
                if ((result = IS_ERR(pci_slot)))
                        goto out;
        }

I've finished reviewing and testing your patches. The rest of your
patch looks good to me. Of course, we must not forget the comment
from Taku Izumi.

Thanks,
Kenji Kaneshige



--

[ message continues ]

Hi Kenji-san,


I'm sorry, I don't think I see the problem you are pointing out.

If pci_get_pci_slot() finds a pci_slot, we do not modify
pci_bus->slots any further, so even if a new slot is created, it
shouldn't affect the pci_slot that we already found.

I must be missing something, but I don't know what. Would you

Yes, I've modified my patch series to take into account the bugs
that Taku-san found.

Thank you both for your reviews.

/ac

--

[ message continues ]

Here is one of the example of the problem I imagine.

When pci_get_pci_slot() does NOT find a pci_slot, pci_hp_register()
calls pci_create_slot(). If another hotplug driver creates slot
between pci_get_pci_slot() and pci_create_slot(), hotplug member of
pci_slot returned from pci_create_slot() is not NULL in the following


In this case, we should return with -EBUSY. But because there is no
check to see if pci->slot is NULL, pci_slot->hotplug will be overwritten
with the new value.

Thanks,
Kenji Kaneshige

--

[ message continues ]