Re: [PATCH] net: pci: Fix hotplug event timeout with shpchp

Bjorn Helgaas
2019-07-02T17:50:46Z

On Tue, Jul 02, 2019 at 01:35:19PM +0000, Miaohe Lin wrote:
> Hotplug a network card would take more than 5 seconds
> in qemu + shpchp scene. It’s because 5 seconds
> delayed_work in func handle_button_press_event with
> case STATIC_STATE. And this will break some
> protocols with timeout within 5 seconds.

I'm dropping this because of the required delay pointed out by Lukas.

If you think we still need to do something here, please clarify the
situation. Are you hot-adding? Hot-swapping? Since you mention a
protocol timeout, I suspect the latter, e.g., maybe you had an
existing device with connections already open, and you want to replace
it with a new device while preserving those open connections?

We do have to preserve the existing user experience, e.g., delays to
allow operators to recover from mistaken latch opens or button
presses. But if we knew more about what you're trying to do, maybe we
could figure out another approach.

> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> drivers/pci/hotplug/shpchp_ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/hotplug/shpchp_ctrl.c b/drivers/pci/hotplug/shpchp_ctrl.c
> index 078003dcde5b..cbb00acaba0d 100644
> --- a/drivers/pci/hotplug/shpchp_ctrl.c
> +++ b/drivers/pci/hotplug/shpchp_ctrl.c
> @@ -478,7 +478,7 @@ static void handle_button_press_event(struct slot *p_slot)
> p_slot->hpc_ops->green_led_blink(p_slot);
> p_slot->hpc_ops->set_attention_status(p_slot, 0);
>
> - queue_delayed_work(p_slot->wq, &p_slot->work, 5*HZ);
> + queue_delayed_work(p_slot->wq, &p_slot->work, 0);
> break;
> case BLINKINGOFF_STATE:
> case BLINKINGON_STATE:
> --
> 2.21.GIT
>