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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190702175040.GA128603@google.com>
Date:   Tue, 2 Jul 2019 12:50:40 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Miaohe Lin <linmiaohe@...wei.com>
Cc:     andy.shevchenko@...il.com, sebott@...ux.ibm.com, lukas@...ner.de,
        gustavo@...eddedor.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, mingfangsen@...wei.com
Subject: Re: [PATCH] net: pci: Fix hotplug event timeout with shpchp

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