From: Greg KH <gregkh@linuxfoundation.org>
To: Jiang Liu <liuj97@gmail.com>
Cc: Yinghai Lu <yinghai@kernel.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Don Dutile <ddutile@redhat.com>, Jiang Liu <jiang.liu@huawei.com>,
Keping Chen <chenkeping@huawei.com>,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 05/19] PCI: correctly flush workqueue when destroy pcie hotplug controller
Date: Tue, 1 May 2012 22:08:33 -0700 [thread overview]
Message-ID: <20120502050833.GC23579@kroah.com> (raw)
In-Reply-To: <1335539820-11232-6-git-send-email-jiang.liu@huawei.com>
On Fri, Apr 27, 2012 at 11:16:46PM +0800, Jiang Liu wrote:
> From: Jiang Liu <jiang.liu@huawei.com>
>
> When destroying a PCIe hotplug controller, all work items associated with
> that controller should be flushed. Function pcie_cleanup_slot() calls
> cancel_delayed_work() and flush_workqueue() to achieve that.
> Function flush_workqueue() will flush all work items already submitted,
> but new work items submitted by those already submitted work items may
> still be in live state when returning from flush_workqueue().
>
> For the extreme case, pciehp driver may expierence following calling path:
> 1) pcie_isr() -> pciehp_handle_xxx() -> queue_interrupt_event()->queue_work()
> 2) interrupt_event_handler() -> handle_button_press_event() ->
> queue_delayed_work()
> 3) pciehp_queue_pushbutton_work() -> queue_work()
>
> So enhance pcie_cleanup_slot() to correctly flush workqueue when destroying
> PCIe hotplug controller.
>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
> drivers/pci/hotplug/pciehp_hpc.c | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index a960fae..98b775f 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -889,8 +889,20 @@ static int pcie_init_slot(struct controller *ctrl)
> static void pcie_cleanup_slot(struct controller *ctrl)
> {
> struct slot *slot = ctrl->slot;
> - cancel_delayed_work(&slot->work);
> +
> + /*
> + * Following workqueue flushing logic is to deal with the special
> + * call path:
> + * 1) pcie_isr() -> pciehp_handle_xxx() ->
> + * queue_interrupt_event(pciehp_wq_event)->queue_work(pciehp_wq)
> + * 2) interrupt_event_handler() -> handle_button_press_event() ->
> + * queue_delayed_work(pciehp_wq)
> + * 3) pciehp_queue_pushbutton_work() -> queue_work(pciehp_wq)
> + */
> flush_workqueue(pciehp_wq);
> + cancel_delayed_work_sync(&slot->work);
> + flush_workqueue(pciehp_wq);
Why just flush it twice? Why not three times?
Like the magic "sync && sync && sync" invocation long typed by
sysadmins.
This really does not feel right, sorry.
Odds are the whole workqueue path needs to be reworked here, right?
greg k-h
next prev parent reply other threads:[~2012-05-02 6:42 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-27 15:16 [PATCH v2 00/19] Introduce a global lock to serialize all PCI hotplug Jiang Liu
2012-04-27 15:16 ` [PATCH v2 01/19] PCI: introduce pci_bus_get()/pci_bus_put() to hide PCI implementation details Jiang Liu
2012-04-27 15:16 ` [PATCH v2 02/19] PCI: introduce recursive rwsem to serialize PCI hotplug operations Jiang Liu
2012-05-02 5:00 ` Greg KH
2012-05-02 7:25 ` Jiang Liu
2012-05-02 21:46 ` Greg KH
2012-04-27 15:16 ` [PATCH v2 03/19] PCI: replace pci_remove_rescan_mutex with the PCI hotplug lock Jiang Liu
2012-04-27 15:16 ` [PATCH v2 04/19] PCI: serialize hotplug operations triggered by PCI hotplug sysfs interfaces Jiang Liu
2012-05-02 5:06 ` Greg KH
2012-05-02 7:20 ` Jiang Liu
2012-05-02 21:48 ` Greg KH
2012-04-27 15:16 ` [PATCH v2 05/19] PCI: correctly flush workqueue when destroy pcie hotplug controller Jiang Liu
2012-05-02 5:08 ` Greg KH [this message]
2012-04-27 15:16 ` [PATCH v2 06/19] PCI: prepare for serializing hotplug operations triggered by pciehp driver Jiang Liu
2012-05-02 5:10 ` Greg KH
2012-04-27 15:16 ` [PATCH v2 07/19] PCI: serialize hotplug operaitons triggered by the " Jiang Liu
2012-04-27 15:16 ` [PATCH v2 08/19] PCI: fix two race windows when probing/removing SHPC controller Jiang Liu
2012-04-27 15:16 ` [PATCH v2 09/19] PCI: correctly flush workqueues and timer when destroy " Jiang Liu
2012-04-27 15:16 ` [PATCH v2 10/19] PCI: serialize hotplug operaitons triggered by the shpchp driver Jiang Liu
2012-04-27 15:16 ` [PATCH v2 11/19] PCI: release IO resource in error handling path in cpcihp_generic_init() Jiang Liu
2012-04-27 15:16 ` [PATCH v2 12/19] PCI: clean up all resources in error handling path in zt5550_hc_init_one() Jiang Liu
2012-04-27 15:16 ` [PATCH v2 13/19] PCI: trivial code clean up in cpci_hotplug_core.c Jiang Liu
2012-04-27 15:16 ` [PATCH v2 14/19] PCI: fix race windows when shutting down cpcihp controller Jiang Liu
2012-04-27 15:16 ` [PATCH v2 15/19] PCI: hold a reference count to the PCI bus used by cpcihp drivers Jiang Liu
2012-04-27 15:16 ` [PATCH v2 16/19] PCI: serialize PCI hotplug operations triggered " Jiang Liu
2012-04-27 15:16 ` [PATCH v2 17/19] PCI: serialize PCI hotplug operations triggered by fakephp drivers Jiang Liu
2012-04-27 15:16 ` [PATCH v2 18/19] PCI, sysfs: Use device_type and attr_groups with pci dev Jiang Liu
2012-04-27 15:17 ` [PATCH v2 19/19] PCI: hide sys interface 'remove' and 'rescan' for SR-IOV virtual devices Jiang Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120502050833.GC23579@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=bhelgaas@google.com \
--cc=chenkeping@huawei.com \
--cc=ddutile@redhat.com \
--cc=jiang.liu@huawei.com \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=liuj97@gmail.com \
--cc=yinghai@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox