From patchwork Tue May 30 09:08:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Srinath Mannam X-Patchwork-Id: 9754139 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9F8B7602BF for ; Tue, 30 May 2017 09:08:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 93A5A27F10 for ; Tue, 30 May 2017 09:08:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 885CC283C0; Tue, 30 May 2017 09:08:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 303BE27F10 for ; Tue, 30 May 2017 09:08:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751347AbdE3JIf (ORCPT ); Tue, 30 May 2017 05:08:35 -0400 Received: from mail-pf0-f171.google.com ([209.85.192.171]:36468 "EHLO mail-pf0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750912AbdE3JId (ORCPT ); Tue, 30 May 2017 05:08:33 -0400 Received: by mail-pf0-f171.google.com with SMTP id m17so66695593pfg.3 for ; Tue, 30 May 2017 02:08:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:to:cc:subject:date:message-id; bh=pvC97HSjb/ooFBXhxT6Z7adQjKp4CPrDg2TGlOzOlBs=; b=emMVEwD9aVbuqOXYrDi/bREzSPTb8JtNa3m6Zqzh5Yp4Lk6OeY1Eaex1C9Nh2cBq0Q +8cIqmRq3IGAeL3zUXfpF5l+NRWEbMOst2vxrEIQkNRt66Xt3wFowhihO+xihGNvtAAj j+MFeem1UMKeGLK4JuZ50Ug5H5b1hwSYV/txg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=pvC97HSjb/ooFBXhxT6Z7adQjKp4CPrDg2TGlOzOlBs=; b=iHS/FkHHs1mj7I0N9EiDm16MA3ch57obcDGBymIviJ4Q5qEhwq/ERbV/crnjo5VmO6 EY6d5yTySdVbhfp8sHw4FRmrh3gZM5xGDVfRNtBh6wstnueyVBQJhXXBZY6nuFFy6/6Z LtZ2sSXkfRu25FqmD3dSE0jBDtuB68hXsTFHX6R3s5hFg6LXODbsm3f4qsSgvZMfoHGE VSOLDllLkZsNx6QkEM3+AaT+BSXfNLgNE2DJs7BLRXDaHnXh+AQnoVizJVCEwrK3LRcu pp4sinnTtjJQyB7gt1vH1EnH7ew4Gjv4d+jpTePVnlAqnQ2GW7cVaza4ce0YGCLWXsh3 03jA== X-Gm-Message-State: AODbwcCAflHQr3Xqcp//zBm1ITJ3oHZ3SJuDgr+JCvAf71pyCmDiywtG +YtqBce2TByy4Wk5Asq0mw== X-Received: by 10.84.184.12 with SMTP id l12mr83128757plc.9.1496135312704; Tue, 30 May 2017 02:08:32 -0700 (PDT) Received: from mannams-OptiPlex-7010.dhcp.avagotech.net ([192.19.237.250]) by smtp.gmail.com with ESMTPSA id z7sm3847276pgr.3.2017.05.30.02.08.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 30 May 2017 02:08:31 -0700 (PDT) From: Srinath Mannam To: bhelgaas@google.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, Srinath Mannam Subject: [RFC PATCH v2] pci: Concurrency issue in NVMe Init through PCIe switch Date: Tue, 30 May 2017 14:38:17 +0530 Message-Id: <1496135297-19680-1-git-send-email-srinath.mannam@broadcom.com> X-Mailer: git-send-email 2.7.4 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We found a concurrency issue in NVMe Init when we initialize multiple NVMe connected over PCIe switch. Setup details: - SMP system with 8 ARMv8 cores running Linux kernel(4.11). - Two NVMe cards are connected to PCIe RC through bridge as shown in the below figure. [RC] | [BRIDGE] | ----------- | | [NVMe] [NVMe] Issue description: After PCIe enumeration completed NVMe driver probe function called for both the devices from two CPUS simultaneously. From nvme_probe, pci_enable_device_mem called for both the EPs. This function called pci_enable_bridge enable recursively until RC. Inside pci_enable_bridge function, at two places concurrency issue is observed. Place 1: CPU 0: 1. Done Atomic increment dev->enable_cnt in pci_enable_device_flags 2. Inside pci_enable_resources 3. Completed pci_read_config_word(dev, PCI_COMMAND, &cmd) 4. Ready to set PCI_COMMAND_MEMORY (0x2) in pci_write_config_word(dev, PCI_COMMAND, cmd) CPU 1: 1. Check pci_is_enabled in function pci_enable_bridge and it is true 2. Check (!dev->is_busmaster) also true 3. Gone into pci_set_master 4. Completed pci_read_config_word(dev, PCI_COMMAND, &old_cmd) 5. Ready to set PCI_COMMAND_MASTER (0x4) in pci_write_config_word(dev, PCI_COMMAND, cmd) By the time of last point for both the CPUs are read value 0 and ready to write 2 and 4. After last point final value in PCI_COMMAND register is 4 instead of 6. Place 2: CPU 0: 1. Done Atomic increment dev->enable_cnt in pci_enable_device_flags Signed-off-by: Srinath Mannam --- Changes since v1: - Used mutex to syncronize pci_enable_bridge drivers/pci/pci.c | 4 ++++ drivers/pci/probe.c | 1 + include/linux/pci.h | 1 + 3 files changed, 6 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b01bd5b..5bff3e7 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1347,7 +1347,9 @@ static void pci_enable_bridge(struct pci_dev *dev) { struct pci_dev *bridge; int retval; + struct mutex *lock = &dev->bridge_lock; + mutex_lock(lock); bridge = pci_upstream_bridge(dev); if (bridge) pci_enable_bridge(bridge); @@ -1355,6 +1357,7 @@ static void pci_enable_bridge(struct pci_dev *dev) if (pci_is_enabled(dev)) { if (!dev->is_busmaster) pci_set_master(dev); + mutex_unlock(lock); return; } @@ -1363,6 +1366,7 @@ static void pci_enable_bridge(struct pci_dev *dev) dev_err(&dev->dev, "Error enabling bridge (%d), continuing\n", retval); pci_set_master(dev); + mutex_unlock(lock); } static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 19c8950..1c25d1c 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -880,6 +880,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, child->dev.parent = child->bridge; pci_set_bus_of_node(child); pci_set_bus_speed(child); + mutex_init(&bridge->bridge_lock); /* Set up default resource pointers and names.. */ for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { diff --git a/include/linux/pci.h b/include/linux/pci.h index 33c2b0b..7e88f41 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -266,6 +266,7 @@ struct pci_dev { void *sysdata; /* hook for sys-specific extension */ struct proc_dir_entry *procent; /* device entry in /proc/bus/pci */ struct pci_slot *slot; /* Physical slot this device is in */ + struct mutex bridge_lock; unsigned int devfn; /* encoded device & function index */ unsigned short vendor;