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

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

[Miaohe Lin]

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.

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

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

[Bjorn Helgaas]

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
> 

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

[linmiaohe]

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 
>
> 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
> 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.

Thanks for your reply. As is spec required, we would try to work around it.
Many thanks.

Have a good day.
Best wishes.