From patchwork Fri Mar 12 04:16:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 398492 Delivered-To: patch@linaro.org Received: by 2002:a02:8562:0:0:0:0:0 with SMTP id g89csp879907jai; Thu, 11 Mar 2021 20:17:43 -0800 (PST) X-Google-Smtp-Source: ABdhPJxpy1uCeLZ0FshYUKPtbNd9ZdNCWIdlJ8Gvcid31F2Fj/Cw3Js2hmHiR727hrL3kX9hLSTk X-Received: by 2002:a05:6402:3487:: with SMTP id v7mr11853531edc.302.1615522662935; Thu, 11 Mar 2021 20:17:42 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1615522662; cv=none; d=google.com; s=arc-20160816; b=LUWrzbJb+/9zW22QSCEiafLq2TZO/XdaW/7S/isBE1G2k8XvG0M/heZhj+aAsK+AEV 0qckkPgv3UL5v4mrmqxOTlNWAx+NFX5FvwtVWEvmf0hAEt60BUu+bIv15C6JS2mK14Li ZHrobVKI5PALHLwynwzMmV1yAAQ3HaK0ADyiy58cH1RBTBmvOht/iupl7ZgFZZrILPX5 rzsZbeRCki5a7gHX8cZv1O6YZFOyk7Q10KTFE6nJLkWmTIPmz1EvFoVHoRiO3oyF0s5m ep0yEq1FDbdzAd7ASOkTS9BemARGj/bkll1ZeIdzyQUkFX7J4TqUOitNWqlL/EXOIKJe Qt+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=psCwVt+HMpuF/3ueFUvJ3Kzy08liF94PegpIe6jUsIw=; b=m8VtqYFcTAO0rp8RXes2NXlCGhCDMYI36EJR+aVSXttdJKoEkCl0Or6451nFYr9L+Z mXuJKI+UXXlT7sFUz7Rfyxb7XYGmp70W5zVOJnTNqu/EW7en9aIL+4kw1UheZBCWJff5 LlFu6PFC1zrZbpuCTnAEXcUC9ZVE6eSLgnfV8myvw1XYbosBALJrgVj78y+qcssBGtN4 9ye5aqXFCuiXbTpchbgsGj6YYoY4OpxTEDUK8mdEkybgBoApK2E/mlHwpR7OTqzu5trX PXryBgogzV7ZSa2x5OzQP2Gn0MMAPt7UrYfcntGwwQHzDjMm/1fCtwxioHvpBij91HrZ L/pQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=k1X7gN79; spf=pass (google.com: domain of linux-pm-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id p26si3023165edt.29.2021.03.11.20.17.38; Thu, 11 Mar 2021 20:17:42 -0800 (PST) Received-SPF: pass (google.com: domain of linux-pm-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=k1X7gN79; spf=pass (google.com: domain of linux-pm-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232122AbhCLERG (ORCPT + 7 others); Thu, 11 Mar 2021 23:17:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40104 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232115AbhCLERF (ORCPT ); Thu, 11 Mar 2021 23:17:05 -0500 Received: from mail-pl1-x635.google.com (mail-pl1-x635.google.com [IPv6:2607:f8b0:4864:20::635]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6125DC061762 for ; Thu, 11 Mar 2021 20:17:05 -0800 (PST) Received: by mail-pl1-x635.google.com with SMTP id d23so8100527plq.2 for ; Thu, 11 Mar 2021 20:17:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=psCwVt+HMpuF/3ueFUvJ3Kzy08liF94PegpIe6jUsIw=; b=k1X7gN79bF4A1DpE1plL6Cx86EhbOg4f5q1LzqlVl0/MzLs9k1G/w/f596T7c0jS3E GF3qr9n0aWxWQ9nj7NcNbQ0lbHY2igl+pl0JnN8QltHk1TdCjTUc0EGUpxEqc7PAZSkZ BHaq0mJ8EAArvE+aXmIPVBJvvoYl15SWgKYcoqqoAZdCNnwBfgHNj+I29Zh9TOVDp8I9 Jo1RXdlFWfgasLPPy+cG/J1c3aSjez8jLMIyNVbQUaZRuRBblbRnNG1lXH0GOdYe7knR 41insHDKPDGpLM6vBt8JJUTBDMQSkJQGpgkJPt4ssiLJ9iZEWqVgoC7hVeS8JxFm9c9k ZFZQ== 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:mime-version :content-transfer-encoding; bh=psCwVt+HMpuF/3ueFUvJ3Kzy08liF94PegpIe6jUsIw=; b=HU1cPYNnX7fCLE5+VfZhZ8LzmojWqs6QNnyf+XVD4jnrBD2cAOK6JNq+oKev/Y3g6s xbAuZjyNPqBeoHs0NPu0i7krnAFmta2mNZkiOJ/lDN0HxsK9iNqRr1IE6Xda6+JEtSlH MV/rhctDumRUh+txOsnwA9jMRlo6EbSxx4HfqmFgVjxDyHqYYK8Iqm9x5cAr3nvZ2FTw CTfEz6LMuuZSmUSWgjpwqf8FCPKHrNY19uNJy3kPFmpyEk0AHOOow7ruHcJmzdKU2gpI wV+5wGbxexpj1zoA1c7+FOMIeomn4Q6L4wnRR6uljsyi/ZwAA32qcOnSblPkuFaldhfc bMag== X-Gm-Message-State: AOAM5332EWTXARmI38jSurtzPOZ5SF/AJ4Jr5ATbOW8uL2nQGXj3iXbk mQ2L8YjHKTnVp5nkFkE7ow2ivQ== X-Received: by 2002:a17:90a:bb02:: with SMTP id u2mr8027389pjr.175.1615522624606; Thu, 11 Mar 2021 20:17:04 -0800 (PST) Received: from localhost ([122.171.124.15]) by smtp.gmail.com with ESMTPSA id b3sm3507535pgd.48.2021.03.11.20.17.02 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Mar 2021 20:17:03 -0800 (PST) From: Viresh Kumar To: Viresh Kumar , Nishanth Menon , Stephen Boyd Cc: Viresh Kumar , linux-pm@vger.kernel.org, Vincent Guittot , Rafael Wysocki , Beata Michalska , "v5 . 11+" , linux-kernel@vger.kernel.org Subject: [PATCH V2] opp: Don't drop extra references to OPPs accidentally Date: Fri, 12 Mar 2021 09:46:59 +0530 Message-Id: <606a5d4227e4610399c61086ac55c46068a90b03.1615522590.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.25.0.rc1.19.g042ed3e048af MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: Beata Michalska We are required to call dev_pm_opp_put() from outside of the opp_table->lock as debugfs removal needs to happen lock-less to avoid circular dependency issues. commit cf1fac943c63 ("opp: Reduce the size of critical section in _opp_kref_release()") tried to fix that introducing a new routine _opp_get_next() which keeps returning OPPs that can be freed by the callers and this routine shall be called without holding the opp_table->lock. Though the commit overlooked the fact that the OPPs can be referenced by other users as well and this routine will end up dropping references which were taken by other users and hence freeing the OPPs prematurely. In effect, other users of the OPPs will end up having invalid pointers at hand. We didn't see any crash reports earlier as the exact situation never happened, though it is certainly possible. We need a way to mark which OPPs are no longer referenced by the OPP core, so we don't drop extra references to them accidentally. This commit adds another OPP flag, "removed", which is used to track this. And now we should never end up dropping extra references to the OPPs. Cc: v5.11+ # v5.11+ Fixes: cf1fac943c63 ("opp: Reduce the size of critical section in _opp_kref_release()") Signed-off-by: Beata Michalska [ Viresh: Almost rewrote entire patch, added new "removed" field, rewrote commit log and added the correct Fixes tag. ] Co-developed-by: Viresh Kumar Signed-off-by: Viresh Kumar --- Sending it formally again so others don't miss it. drivers/opp/core.c | 48 ++++++++++++++++++++++++---------------------- drivers/opp/opp.h | 2 ++ 2 files changed, 27 insertions(+), 23 deletions(-) -- 2.25.0.rc1.19.g042ed3e048af diff --git a/drivers/opp/core.c b/drivers/opp/core.c index c2689386a906..1556998425d5 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1492,7 +1492,11 @@ static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table, mutex_lock(&opp_table->lock); list_for_each_entry(temp, &opp_table->opp_list, node) { - if (dynamic == temp->dynamic) { + /* + * Refcount must be dropped only once for each OPP by OPP core, + * do that with help of "removed" flag. + */ + if (!temp->removed && dynamic == temp->dynamic) { opp = temp; break; } @@ -1502,10 +1506,27 @@ static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table, return opp; } -bool _opp_remove_all_static(struct opp_table *opp_table) +/* + * Can't call dev_pm_opp_put() from under the lock as debugfs removal needs to + * happen lock less to avoid circular dependency issues. This routine must be + * called without the opp_table->lock held. + */ +static void _opp_remove_all(struct opp_table *opp_table, bool dynamic) { struct dev_pm_opp *opp; + while ((opp = _opp_get_next(opp_table, dynamic))) { + opp->removed = true; + dev_pm_opp_put(opp); + + /* Drop the references taken by dev_pm_opp_add() */ + if (dynamic) + dev_pm_opp_put_opp_table(opp_table); + } +} + +bool _opp_remove_all_static(struct opp_table *opp_table) +{ mutex_lock(&opp_table->lock); if (!opp_table->parsed_static_opps) { @@ -1520,13 +1541,7 @@ bool _opp_remove_all_static(struct opp_table *opp_table) mutex_unlock(&opp_table->lock); - /* - * Can't remove the OPP from under the lock, debugfs removal needs to - * happen lock less to avoid circular dependency issues. - */ - while ((opp = _opp_get_next(opp_table, false))) - dev_pm_opp_put(opp); - + _opp_remove_all(opp_table, false); return true; } @@ -1539,25 +1554,12 @@ bool _opp_remove_all_static(struct opp_table *opp_table) void dev_pm_opp_remove_all_dynamic(struct device *dev) { struct opp_table *opp_table; - struct dev_pm_opp *opp; - int count = 0; opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return; - /* - * Can't remove the OPP from under the lock, debugfs removal needs to - * happen lock less to avoid circular dependency issues. - */ - while ((opp = _opp_get_next(opp_table, true))) { - dev_pm_opp_put(opp); - count++; - } - - /* Drop the references taken by dev_pm_opp_add() */ - while (count--) - dev_pm_opp_put_opp_table(opp_table); + _opp_remove_all(opp_table, true); /* Drop the reference taken by _find_opp_table() */ dev_pm_opp_put_opp_table(opp_table); diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 50fb9dced3c5..407c3bfe51d9 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -56,6 +56,7 @@ extern struct list_head opp_tables, lazy_opp_tables; * @dynamic: not-created from static DT entries. * @turbo: true if turbo (boost) OPP * @suspend: true if suspend OPP + * @removed: flag indicating that OPP's reference is dropped by OPP core. * @pstate: Device's power domain's performance state. * @rate: Frequency in hertz * @level: Performance level @@ -78,6 +79,7 @@ struct dev_pm_opp { bool dynamic; bool turbo; bool suspend; + bool removed; unsigned int pstate; unsigned long rate; unsigned int level;