Re: [PATCH] pciehp: Handle interrupts that happen during initialization.

From: Kenji Kaneshige
Date: Fri Feb 20 2009 - 01:18:44 EST


Eric W. Biederman wrote:
> Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> writes:
>
>> In the current pciehp implementation, minimum resources enough to
>> enable devices under the bridge are assigned when P2P bridge is
>> hot-added. My concern is that enough resources are NOT assigned to
>> the bridge if an additional slot is empty. As a result, hot-add
>> adapter card on the additional slot won't work because of resource
>> shortage.
>
> It is a good concern. Right now I know I won't need a bus number
> but you are quite right the mmio and iospace may be a problem.
>
> My preliminary test case doesn't cover that so I will look and
> confirm it is a problem I need to address.
>
>>> kobject_name is not initialized, and slot_name(p_slot) calls
>>> hoptlug_slot_name which calls pci_slot_name which kobj_name.
>>> It looks like this problem was introduced in commit
>>> e1acb24f059defdaa0264e925f19cc21b0a3e592
>> Thank your for the information. I understood what is happening.
>> This needs to be fixed. But, as I mentioned before, I think
>> software notification mechanism should be initialized before
>> sysfs entries are created. I'll consider alternative fix.
>
> Reasonable. I haven't had the need nor gotten brave enough to
> support those sysfs entries in my minimal driver.
>

Eric-san, Jesse-san,

I tried to make alternative fix, but now I have a conclusion that
Eric-san's patch is the best solution. The reasons are

- Eric-san's approach prevents the regression such as commit
e1acb24f059defdaa0264e925f19cc21b0a3e592.

- My concern was that command completion might not be handled if
we enable software notification mechanism after creating sysfs
entries. But it is not true because pciehp uses polling approach
if command is issued before enabling software notification (I
made it, but I forgot it...). Just in case, I confirmed that
hotplug operations (power on/off, attention indication on/off
and so on) works even if I make pciehp_init_notification() NOP.

Jesse-san, could you consider applying Eric-san's patch?

Here are my

Reviewed-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>
Tested-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>

Note: I tested Eric-san's patch on Jesse-san's for-linus tree.

Thanks,
Kenji Kaneshige


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/