From patchwork Tue Feb 11 02:22:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rajat Jain X-Patchwork-Id: 3624631 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3DC039F2F6 for ; Tue, 11 Feb 2014 02:22:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 02C73201F7 for ; Tue, 11 Feb 2014 02:22:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DC423201F2 for ; Tue, 11 Feb 2014 02:22:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752133AbaBKCWm (ORCPT ); Mon, 10 Feb 2014 21:22:42 -0500 Received: from mail-pa0-f53.google.com ([209.85.220.53]:38649 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752047AbaBKCWl (ORCPT ); Mon, 10 Feb 2014 21:22:41 -0500 Received: by mail-pa0-f53.google.com with SMTP id lj1so7011290pab.12 for ; Mon, 10 Feb 2014 18:22:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=4Gwh+HLaYnqgu7jkPFX7CqNgiUIGvY7FutRNEv5qHhs=; b=Idg2BSSYo0wf6tH3sOCgBlwquWV2V60pyahdnQc0vgo51ScHSU7CgqqFp7y2uBVbct 1sfSxdegribZhUM/T43wabEFA5WON3Y1vmR+4TU8N/EVqL2jpfIFwMvvIvmy8bvdrDL2 xazQFwoVXHksHBt9iJwx6Hs7HPmINGy1gHa4Qqd7WcYO0YJruRuBcpvx5We0t3v/NEAx JUI+ro+yAaiAC72eBTzcJlQSuzySQsEptzdnYzVnAcU4eISE6RAfRmcPNP7kIwy/NzVW PDhmQxWnSeGALWuoUGdCHN8Jb2dj1HYf0qMbj2T+uTiHhDOsJfU25cDKczKBMHO1h2a3 CWNQ== X-Received: by 10.68.171.99 with SMTP id at3mr41318848pbc.109.1392085361191; Mon, 10 Feb 2014 18:22:41 -0800 (PST) Received: from [192.168.211.137] ([66.129.239.14]) by mx.google.com with ESMTPSA id tu3sm124303427pab.1.2014.02.10.18.22.39 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 10 Feb 2014 18:22:40 -0800 (PST) Message-ID: <52F9896D.7090908@gmail.com> Date: Mon, 10 Feb 2014 18:22:37 -0800 From: Rajat Jain User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130803 Thunderbird/17.0.8 MIME-Version: 1.0 To: Bjorn Helgaas CC: Rajat Jain , "Rafael J. Wysocki" , Kenji Kaneshige , Alex Williamson , Yijing Wang , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Yinghai Lu , Guenter Roeck Subject: [PATCH v5 8/8] Allow Link state changes for Hot-Plug References: <52F1A1B6.2090709@gmail.com> <4f93bb24b6214bfb9dbcc91b682184eb@DM2PR05MB671.namprd05.prod.outlook.com> <20140211011816.GA15412@google.com> In-Reply-To: <20140211011816.GA15412@google.com> Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Today it is there is no protection around pciehp_enable_slot() and pciehp_disable_slot() to ensure that they complete before another hot-plug operation can be done on that particular slot. This patch introduces the slot->hotplug_lock to ensure that any hotplug operations (add / remove) complete before another HP event can begin processing on that particular slot. Signed-off-by: Rajat Jain Signed-off-by: Rajat Jain Signed-off-by: Guenter Roeck --- Hello Bjorn, > > I applied these to pci/pciehp for v3.15, thanks! I dropped the "ret" decl > in 8/8, which I assume was the cause of the unused variable warning. > Thank you so much for accepting the patchset. The "ret" variable in 8/8 was supposed to be used for holding the return value, so that the call to pciehp_green_led_off() could be moved to after releasing the lock. Here is the revised 8/8 patch (only a couple lines diff from v4). Please feel free to either use this, or just make that small change in pciehp_power_thread(). I was holding off sending this in case there were any other comments. Thank you so much once again, Rajat v5: Remove the "unused variable" warning by using it to hold return value (Thus reducing lock hold time, by moving pciehp_green_led_off() to after releasing the lock) v4: same as v3, only rebased on top of 3.14-rc1 v3: same as v2, only patch numbering changed from [4/4] to [8/8]. v2: Same as v1, dropped the "RFC" from subject, added Guenter's signature v1: Initial patch drivers/pci/hotplug/pciehp.h | 1 + drivers/pci/hotplug/pciehp_core.c | 7 ++++++- drivers/pci/hotplug/pciehp_ctrl.c | 17 +++++++++++++++-- drivers/pci/hotplug/pciehp_hpc.c | 1 + 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index d8d0336..8a66866 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h @@ -76,6 +76,7 @@ struct slot { struct hotplug_slot *hotplug_slot; struct delayed_work work; /* work for button event */ struct mutex lock; + struct mutex hotplug_lock; struct workqueue_struct *wq; }; diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index 53b58de..23b4bde 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -283,8 +283,11 @@ static int pciehp_probe(struct pcie_device *dev) slot = ctrl->slot; pciehp_get_adapter_status(slot, &occupied); pciehp_get_power_status(slot, &poweron); - if (occupied && pciehp_force) + if (occupied && pciehp_force) { + mutex_lock(&slot->hotplug_lock); pciehp_enable_slot(slot); + mutex_unlock(&slot->hotplug_lock); + } /* If empty slot's power status is on, turn power off */ if (!occupied && poweron && POWER_CTRL(ctrl)) pciehp_power_off_slot(slot); @@ -328,10 +331,12 @@ static int pciehp_resume (struct pcie_device *dev) /* Check if slot is occupied */ pciehp_get_adapter_status(slot, &status); + mutex_lock(&slot->hotplug_lock); if (status) pciehp_enable_slot(slot); else pciehp_disable_slot(slot); + mutex_unlock(&slot->hotplug_lock); return 0; } #endif /* PM */ diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 3e40ec0..fec99a1 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -293,6 +293,7 @@ static void pciehp_power_thread(struct work_struct *work) struct power_work_info *info = container_of(work, struct power_work_info, work); struct slot *p_slot = info->p_slot; + int ret; switch (info->req) { case DISABLE_REQ: @@ -300,7 +301,9 @@ static void pciehp_power_thread(struct work_struct *work) "Disabling domain:bus:device=%04x:%02x:00\n", pci_domain_nr(p_slot->ctrl->pcie->port->subordinate), p_slot->ctrl->pcie->port->subordinate->number); + mutex_lock(&p_slot->hotplug_lock); pciehp_disable_slot(p_slot); + mutex_unlock(&p_slot->hotplug_lock); mutex_lock(&p_slot->lock); p_slot->state = STATIC_STATE; mutex_unlock(&p_slot->lock); @@ -310,7 +313,10 @@ static void pciehp_power_thread(struct work_struct *work) "Enabling domain:bus:device=%04x:%02x:00\n", pci_domain_nr(p_slot->ctrl->pcie->port->subordinate), p_slot->ctrl->pcie->port->subordinate->number); - if (pciehp_enable_slot(p_slot)) + mutex_lock(&p_slot->hotplug_lock); + ret = pciehp_enable_slot(p_slot); + mutex_unlock(&p_slot->hotplug_lock); + if (ret) pciehp_green_led_off(p_slot); mutex_lock(&p_slot->lock); p_slot->state = STATIC_STATE; @@ -546,6 +552,9 @@ static void interrupt_event_handler(struct work_struct *work) kfree(info); } +/* + * Note: This function must be called with slot->hotplug_lock held + */ int pciehp_enable_slot(struct slot *p_slot) { u8 getstatus = 0; @@ -584,7 +593,9 @@ int pciehp_enable_slot(struct slot *p_slot) return rc; } - +/* + * Note: This function must be called with slot->hotplug_lock held + */ int pciehp_disable_slot(struct slot *p_slot) { u8 getstatus = 0; @@ -617,7 +628,9 @@ int pciehp_sysfs_enable_slot(struct slot *p_slot) case STATIC_STATE: p_slot->state = POWERON_STATE; mutex_unlock(&p_slot->lock); + mutex_lock(&p_slot->hotplug_lock); retval = pciehp_enable_slot(p_slot); + mutex_unlock(&p_slot->hotplug_lock); mutex_lock(&p_slot->lock); p_slot->state = STATIC_STATE; break; diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 55cc389..68447a1 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -686,6 +686,7 @@ static int pcie_init_slot(struct controller *ctrl) slot->ctrl = ctrl; mutex_init(&slot->lock); + mutex_init(&slot->hotplug_lock); INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work); ctrl->slot = slot; return 0;