lkml.org 
[lkml]   [2022]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 429/667] nvdimm: Fix firmware activation deadlock scenarios
    Date
    From: Dan Williams <dan.j.williams@intel.com>

    [ Upstream commit e6829d1bd3c4b58296ee9e412f7ed4d6cb390192 ]

    Lockdep reports the following deadlock scenarios for CXL root device
    power-management, device_prepare(), operations, and device_shutdown()
    operations for 'nd_region' devices:

    Chain exists of:
    &nvdimm_region_key --> &nvdimm_bus->reconfig_mutex --> system_transition_mutex

    Possible unsafe locking scenario:

    CPU0 CPU1
    ---- ----
    lock(system_transition_mutex);
    lock(&nvdimm_bus->reconfig_mutex);
    lock(system_transition_mutex);
    lock(&nvdimm_region_key);

    Chain exists of:
    &cxl_nvdimm_bridge_key --> acpi_scan_lock --> &cxl_root_key

    Possible unsafe locking scenario:

    CPU0 CPU1
    ---- ----
    lock(&cxl_root_key);
    lock(acpi_scan_lock);
    lock(&cxl_root_key);
    lock(&cxl_nvdimm_bridge_key);

    These stem from holding nvdimm_bus_lock() over hibernate_quiet_exec()
    which walks the entire system device topology taking device_lock() along
    the way. The nvdimm_bus_lock() is protecting against unregistration,
    multiple simultaneous ops callers, and preventing activate_show() from
    racing activate_store(). For the first 2, the lock is redundant.
    Unregistration already flushes all ops users, and sysfs already prevents
    multiple threads to be active in an ops handler at the same time. For
    the last userspace should already be waiting for its last
    activate_store() to complete, and does not need activate_show() to flush
    the write side, so this lock usage can be deleted in these attributes.

    Fixes: 48001ea50d17 ("PM, libnvdimm: Add runtime firmware activation support")
    Reviewed-by: Ira Weiny <ira.weiny@intel.com>
    Link: https://lore.kernel.org/r/165074883800.4116052.10737040861825806582.stgit@dwillia2-desk3.amr.corp.intel.com
    Signed-off-by: Dan Williams <dan.j.williams@intel.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/nvdimm/core.c | 9 ---------
    1 file changed, 9 deletions(-)

    diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
    index 7de592d7eff4..47625fe4276e 100644
    --- a/drivers/nvdimm/core.c
    +++ b/drivers/nvdimm/core.c
    @@ -399,9 +399,7 @@ static ssize_t capability_show(struct device *dev,
    if (!nd_desc->fw_ops)
    return -EOPNOTSUPP;

    - nvdimm_bus_lock(dev);
    cap = nd_desc->fw_ops->capability(nd_desc);
    - nvdimm_bus_unlock(dev);

    switch (cap) {
    case NVDIMM_FWA_CAP_QUIESCE:
    @@ -426,10 +424,8 @@ static ssize_t activate_show(struct device *dev,
    if (!nd_desc->fw_ops)
    return -EOPNOTSUPP;

    - nvdimm_bus_lock(dev);
    cap = nd_desc->fw_ops->capability(nd_desc);
    state = nd_desc->fw_ops->activate_state(nd_desc);
    - nvdimm_bus_unlock(dev);

    if (cap < NVDIMM_FWA_CAP_QUIESCE)
    return -EOPNOTSUPP;
    @@ -474,7 +470,6 @@ static ssize_t activate_store(struct device *dev,
    else
    return -EINVAL;

    - nvdimm_bus_lock(dev);
    state = nd_desc->fw_ops->activate_state(nd_desc);

    switch (state) {
    @@ -492,7 +487,6 @@ static ssize_t activate_store(struct device *dev,
    default:
    rc = -ENXIO;
    }
    - nvdimm_bus_unlock(dev);

    if (rc == 0)
    rc = len;
    @@ -515,10 +509,7 @@ static umode_t nvdimm_bus_firmware_visible(struct kobject *kobj, struct attribut
    if (!nd_desc->fw_ops)
    return 0;

    - nvdimm_bus_lock(dev);
    cap = nd_desc->fw_ops->capability(nd_desc);
    - nvdimm_bus_unlock(dev);
    -
    if (cap < NVDIMM_FWA_CAP_QUIESCE)
    return 0;

    --
    2.35.1


    \
     
     \ /
      Last update: 2022-06-07 21:33    [W:4.034 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site