From patchwork Wed Apr 22 21:54:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11504669 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1A4FF92C for ; Wed, 22 Apr 2020 21:55:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0215B22205 for ; Wed, 22 Apr 2020 21:55:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="JG4Xp5iA" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726168AbgDVVz1 (ORCPT ); Wed, 22 Apr 2020 17:55:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58552 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726006AbgDVVz0 (ORCPT ); Wed, 22 Apr 2020 17:55:26 -0400 Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 67BCCC03C1AA for ; Wed, 22 Apr 2020 14:55:25 -0700 (PDT) Received: by mail-pg1-x544.google.com with SMTP id j7so1770745pgj.13 for ; Wed, 22 Apr 2020 14:55:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=VcN3ZrHWXlIPIuh298G1lUnACYhPjzKzp7jscayIaS4=; b=JG4Xp5iAXo+CrWZdn/y4r+OZsOjzrMLbL1ltQWdthqarbD2q+yMDBjC/j/8ONTKO67 izd7bl01J9AylLWiyA8vjPLcbySNq6/vcVFp/ymDArLe3+jWLxvoIZvbXLOiQsHhbcg5 pGgNdAykwkPFFmQVqht88gJI6UEnach0gbPYs= 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=VcN3ZrHWXlIPIuh298G1lUnACYhPjzKzp7jscayIaS4=; b=EZKo/0NcHy1QPMR0zMI7I/Cy8BXwKZ+71OiTrc4yotdCEbSmBJwcfWx+a4xMcXeOKj 5u1jUoSxJSCgACajJO3RSu5ksmNw/4wy9WIKVAn1YN/oppcirTu3yTs8w/oSPdiW2nq2 63M9/doPC1SbwvKwK8GgySg+ZWsuf7KW+aUFlmcN/0fY+jSjwB97bn5d2xZj8wD4q36U I+1C92SYmWs3HoPEURmOlxuysGv2571tlB6hIPfOWx0UYfgSqpUPQEl/zNGDhBK8/Zdp 42gloIUsJf3xwymf6wNZ4j9yztKjfpsIuNPfIMgjEYfN0OOWNE5qjsxJltfc/oRXoGnm YLdQ== X-Gm-Message-State: AGi0PuYuzuREfs3wQQxzknZVap6HYNE5z9aEZMIp8S5ATuhHa3nCCWQZ +zR8nGxjqVTTmHIDHpiDYTlhcw== X-Google-Smtp-Source: APiQypIPMIRi0bHMPdu9jfpd4s1/+Acupprv6NYFFFx0FXOFJvQM+mh4MegAtGry2e5FVLcca2uxgw== X-Received: by 2002:a62:8202:: with SMTP id w2mr594501pfd.117.1587592524740; Wed, 22 Apr 2020 14:55:24 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id r4sm211072pgi.6.2020.04.22.14.55.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Apr 2020 14:55:24 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , rafael.j.wysocki@intel.com, Andy Gross , Bjorn Andersson Cc: mka@chromium.org, swboyd@chromium.org, mkshah@codeaurora.org, evgreen@chromium.org, Douglas Anderson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/5] soc: qcom: rpmh-rsc: Corrently ignore CPU_CLUSTER_PM notifications Date: Wed, 22 Apr 2020 14:54:59 -0700 Message-Id: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog MIME-Version: 1.0 Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Our switch statement doesn't have entries for CPU_CLUSTER_PM_ENTER, CPU_CLUSTER_PM_ENTER_FAILED, and CPU_CLUSTER_PM_EXIT and doesn't have a default. This means that we'll try to do a flush in those cases but we won't necessarily be the last CPU down. That's not so ideal since our (lack of) locking assumes we're on the last CPU. Luckily this isn't as big a problem as you'd think since (at least on the SoC I tested) we don't get these notifications except on full system suspend. ...and on full system suspend we get them on the last CPU down. That means that the worst problem we hit is flushing twice. Still, it's good to make it correct. Fixes: 985427f997b6 ("soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches") Reported-by: Stephen Boyd Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Reviewed-by: Stephen Boyd --- Changes in v4: - ("...Corrently ignore CPU_CLUSTER_PM notifications") split out for v4. Changes in v3: None Changes in v2: None drivers/soc/qcom/rpmh-rsc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index a9e15699f55f..3571a99fc839 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -806,6 +806,8 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb, case CPU_PM_EXIT: cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); goto exit; + default: + return NOTIFY_DONE; } ret = rpmh_rsc_ctrlr_is_busy(drv); From patchwork Wed Apr 22 21:55:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11504671 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 418BD92C for ; Wed, 22 Apr 2020 21:55:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 29DFA2168B for ; Wed, 22 Apr 2020 21:55:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="lMzLpPm6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726228AbgDVVz2 (ORCPT ); Wed, 22 Apr 2020 17:55:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726173AbgDVVz1 (ORCPT ); Wed, 22 Apr 2020 17:55:27 -0400 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 43822C03C1AB for ; Wed, 22 Apr 2020 14:55:26 -0700 (PDT) Received: by mail-pf1-x443.google.com with SMTP id v63so1812449pfb.10 for ; Wed, 22 Apr 2020 14:55:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=HfuhF99BRaSEh0bfmWqr7aSzcTnP1RLVseb4QVvFXi8=; b=lMzLpPm6h+VTQ1aXb2wCKeBILA8eBpvnCjaMmppBWioTLLm56Fars7TegBE981Y4qa UOdGk+wGv/kTK/aZzPeroP9IgG2TaeCkUoP1xJdMwgQA8t3RJcynzQndzKK/IT/4kFLc GPomLauJym/7f+mCx5tyPC+qCW8zVmC1mK/kg= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=HfuhF99BRaSEh0bfmWqr7aSzcTnP1RLVseb4QVvFXi8=; b=HmUoCEtxDLcNbGF0cSYbUb9K0dlGVCPMde7xTFfFO/m+ns3EPIpbRbjLeW+MKclodH DP+0OwWYmRnOTG7067d6NjgHboWlr6qdbT6368DXDygV8NzNlWkv1TmOkL2LjQIXLg3M kKDjhxyonSgIEyBpIlxk1QSbnaELC37uIUOfzwgOvEtxCgWaRpGJ3FuicnItV56x6Cf2 AF8WexJvHhmKX4Vpmr3cXEtWqsFWe4M8jlPbDYp+dTbDynwXZ2aV7SUS9Ioh3YC3esa7 nVSwzikqC3jsW3jWuC6r4Nk6Wl4hBO5MSzpZIVHsTzRvynO3UByfaFuFNDQMjOPd4co/ toMg== X-Gm-Message-State: AGi0PubmOFA+GNsl1Pf7XD32bPNOxu3SwZTjyTWqjmY7T6VGM6lrGb+q FVcESW87koIz9TJB4+EF9OVRwg== X-Google-Smtp-Source: APiQypIMZx29EYw/LuKiakgkcK5iD2peotdc67FckTndEWYZzYUjvYUqWxgImzq3lAvbHw2Tfn3gYw== X-Received: by 2002:a63:df42:: with SMTP id h2mr1118598pgj.216.1587592525841; Wed, 22 Apr 2020 14:55:25 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id r4sm211072pgi.6.2020.04.22.14.55.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Apr 2020 14:55:25 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , rafael.j.wysocki@intel.com, Andy Gross , Bjorn Andersson Cc: mka@chromium.org, swboyd@chromium.org, mkshah@codeaurora.org, evgreen@chromium.org, Douglas Anderson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 2/5] soc: qcom: rpmh-rsc: We aren't notified of our own failure w/ NOTIFY_BAD Date: Wed, 22 Apr 2020 14:55:00 -0700 Message-Id: <20200422145408.v4.2.I1927d1bca2569a27b2d04986baf285027f0818a2@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> References: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> MIME-Version: 1.0 Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org When a PM Notifier returns NOTIFY_BAD it doesn't get called with CPU_PM_ENTER_FAILED. It only get called for CPU_PM_ENTER_FAILED if someone else (further down the notifier chain) returns NOTIFY_BAD. Handle this case by taking our CPU out of the list of ones that have entered PM. Without this it's possible we could detect that the last CPU went down (and we would flush) even if some CPU was alive. That's not good since our flushing routines currently assume they're running on the last CPU for mutual exclusion. Fixes: 985427f997b6 ("soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches") Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Reported-by: Stephen Boyd Reviewed-by: Stephen Boyd --- Changes in v4: - ("...We aren't notified of our own failure...") split out for v4. Changes in v3: None Changes in v2: None drivers/soc/qcom/rpmh-rsc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 3571a99fc839..e540e49fd61c 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -823,6 +823,10 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb, ret = NOTIFY_OK; exit: + if (ret == NOTIFY_BAD) + /* We won't be called w/ CPU_PM_ENTER_FAILED */ + cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); + spin_unlock(&drv->pm_lock); return ret; } From patchwork Wed Apr 22 21:55:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11504675 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A9BD792C for ; Wed, 22 Apr 2020 21:55:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8C2A521D7B for ; Wed, 22 Apr 2020 21:55:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="FoCM56Su" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726109AbgDVVzb (ORCPT ); Wed, 22 Apr 2020 17:55:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58568 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726173AbgDVVz2 (ORCPT ); Wed, 22 Apr 2020 17:55:28 -0400 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B36B6C03C1A9 for ; Wed, 22 Apr 2020 14:55:28 -0700 (PDT) Received: by mail-pj1-x1043.google.com with SMTP id e6so1565098pjt.4 for ; Wed, 22 Apr 2020 14:55:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=j2VzB2lC7n70zeZvI1z98RI+OPNujOUDIg4RGVhNGJY=; b=FoCM56SuzbNrGHRUpV5siD/+XQVMGTtfQ7PhJyibsFVQPV9tve2ZKqBcitt9ge9w9O GZHqEeyY7auFJq13ywcH14FcTdCo18INH3SnsBWSjPJrJOR12FAY3f6SOcHaZJ618xKB ZEiurfDkawixDJdUnIG2QmdC4yWZ/CM7mZpE8= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=j2VzB2lC7n70zeZvI1z98RI+OPNujOUDIg4RGVhNGJY=; b=KeckcmrgK/UeVGqKZRld9Q7T9eanz0G8LbKY53YhPafO+zq8YDE1r2bD/c861qoqOt 8zZK3PnLJ8Lmu5msXFr6ZH9Sk3y9rY/IYJH3RjPA3BvMoOXCUvHYiv/VUa0xPHbgS6Ev smWL0jpdGMLvfQEQ1wih4sw+1pcYx4lHE2DtJGiORJ9vwUm+kNKikPHxgJ0akn5jlK/u nDWYroZapzOCQ+PSRl5P9yddGYGcMnaCYUsx8hiT2L5S9TnSA27DMRat6f2f+M4rVNtM z38BPT2a1uFrD2tLXbmV1+TDm0T3I43DUmE47ng503HoNmAhyMC+2rTM+DSYbCoQFWQo gwqg== X-Gm-Message-State: AGi0PuahT1b6mx9EY+uAoD1rzOkskzAoax5ELfFosXYIKmRFZ3bcF0Ll qxr2kZE0iXW0QirQkfrms0TCvw== X-Google-Smtp-Source: APiQypLv7GUjGug9I6Sz7L3n6p5lN2ljXG2fZBcmM2eBqz+Po43CHZ9dlKjCaNFBly/N6DDTbgfXHw== X-Received: by 2002:a17:90a:23e2:: with SMTP id g89mr848331pje.105.1587592528043; Wed, 22 Apr 2020 14:55:28 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id r4sm211072pgi.6.2020.04.22.14.55.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Apr 2020 14:55:27 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , rafael.j.wysocki@intel.com, Andy Gross , Bjorn Andersson Cc: mka@chromium.org, swboyd@chromium.org, mkshah@codeaurora.org, evgreen@chromium.org, Douglas Anderson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 4/5] soc: qcom: rpmh-rsc: Simplify locking by eliminating the per-TCS lock Date: Wed, 22 Apr 2020 14:55:02 -0700 Message-Id: <20200422145408.v4.4.Ib8dccfdb10bf6b1fb1d600ca1c21d9c0db1ef746@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> References: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> MIME-Version: 1.0 Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org The rpmh-rsc code had both a driver-level lock (sometimes referred to in comments as drv->lock) and a lock per-TCS. The idea was supposed to be that there would be times where you could get by with just locking a TCS lock and therefor other RPMH users wouldn't be blocked. The above didn't work out so well. Looking at tcs_write() the bigger drv->lock was held for most of the function anyway. Only the __tcs_buffer_write() and __tcs_set_trigger() calls were called without it the drv->lock. It actually turns out that in tcs_write() we don't need to hold the drv->lock for those function calls anyway even if the per-TCS lock isn't there anymore. Thus, from a tcs_write() point of view, the per-TCS lock was useless. Looking at rpmh_rsc_write_ctrl_data(), only the per-TCS lock was held. It turns out, though, that this function already needs to be called with the equivalent of the drv->lock held anyway (we either need to hold drv->lock as we will in a future patch or we need to know no other CPUs could be running as happens today). Specifically rpmh_rsc_write_ctrl_data() might be writing to a TCS that has been borrowed for writing an active transation but it never checks this. Let's eliminate this extra overhead and avoid possible AB BA locking headaches. Suggested-by: Maulik Shah Signed-off-by: Douglas Anderson --- Changes in v4: None Changes in v3: - ("soc: qcom: rpmh-rsc: Simplify locking...") new for v3. Changes in v2: None drivers/soc/qcom/rpmh-internal.h | 13 ++------ drivers/soc/qcom/rpmh-rsc.c | 54 ++++++++++++++------------------ 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index dba8510c0669..1f2857b3f38e 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -28,7 +28,6 @@ struct rsc_drv; * @offset: Start of the TCS group relative to the TCSes in the RSC. * @num_tcs: Number of TCSes in this type. * @ncpt: Number of commands in each TCS. - * @lock: Lock for synchronizing this TCS writes. * @req: Requests that are sent from the TCS; only used for ACTIVE_ONLY * transfers (could be on a wake/sleep TCS if we are borrowing for * an ACTIVE_ONLY transfer). @@ -48,7 +47,6 @@ struct tcs_group { u32 offset; int num_tcs; int ncpt; - spinlock_t lock; const struct tcs_request *req[MAX_TCS_PER_TYPE]; DECLARE_BITMAP(slots, MAX_TCS_SLOTS); }; @@ -103,14 +101,9 @@ struct rpmh_ctrlr { * @tcs_in_use: S/W state of the TCS; only set for ACTIVE_ONLY * transfers, but might show a sleep/wake TCS in use if * it was borrowed for an active_only transfer. You - * must hold both the lock in this struct and the - * tcs_lock for the TCS in order to mark a TCS as - * in-use, but you only need the lock in this structure - * (aka the drv->lock) to mark one freed. - * @lock: Synchronize state of the controller. If you will be - * grabbing this lock and a tcs_lock at the same time, - * grab the tcs_lock first so we always have a - * consistent lock ordering. + * must hold the lock in this struct (AKA drv->lock) in + * order to update this. + * @lock: Synchronize state of the controller. * @pm_lock: Synchronize during PM notifications. * Used when solver mode is not present. * @client: Handle to the DRV's client. diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index e540e49fd61c..71cebe7fd452 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -179,11 +179,7 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id, * * Returns true if nobody has claimed this TCS (by setting tcs_in_use). * - * Context: Must be called with the drv->lock held or the tcs_lock for the TCS - * being tested. If only the tcs_lock is held then it is possible that - * this function will return that a tcs is still busy when it has been - * recently been freed but it will never return free when a TCS is - * actually in use. + * Context: Must be called with the drv->lock held. * * Return: true if the given TCS is free. */ @@ -242,8 +238,6 @@ void rpmh_rsc_invalidate(struct rsc_drv *drv) * This is normally pretty straightforward except if we are trying to send * an ACTIVE_ONLY message but don't have any active_only TCSes. * - * Called without drv->lock held and with no tcs_lock locks held. - * * Return: A pointer to a tcs_group or an ERR_PTR. */ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, @@ -581,24 +575,19 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) if (IS_ERR(tcs)) return PTR_ERR(tcs); - spin_lock_irqsave(&tcs->lock, flags); - spin_lock(&drv->lock); + spin_lock_irqsave(&drv->lock, flags); /* * The h/w does not like if we send a request to the same address, * when one is already in-flight or being processed. */ ret = check_for_req_inflight(drv, tcs, msg); - if (ret) { - spin_unlock(&drv->lock); - goto done_write; - } + if (ret) + goto err; - tcs_id = find_free_tcs(tcs); - if (tcs_id < 0) { - ret = tcs_id; - spin_unlock(&drv->lock); - goto done_write; - } + ret = find_free_tcs(tcs); + if (ret < 0) + goto err; + tcs_id = ret; tcs->req[tcs_id - tcs->offset] = msg; set_bit(tcs_id, drv->tcs_in_use); @@ -612,13 +601,21 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, tcs_id, 0); enable_tcs_irq(drv, tcs_id, true); } - spin_unlock(&drv->lock); + spin_unlock_irqrestore(&drv->lock, flags); + /* + * These two can be done after the lock is released because: + * - We marked "tcs_in_use" under lock. + * - Once "tcs_in_use" has been marked nobody else could be writing + * to these registers until the interrupt goes off. + * - The interrupt can't go off until we trigger. + */ __tcs_buffer_write(drv, tcs_id, 0, msg); __tcs_set_trigger(drv, tcs_id, true); -done_write: - spin_unlock_irqrestore(&tcs->lock, flags); + return 0; +err: + spin_unlock_irqrestore(&drv->lock, flags); return ret; } @@ -673,8 +670,6 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg) * Only for use on sleep/wake TCSes since those are the only ones we maintain * tcs->slots for. * - * Must be called with the tcs_lock for the group held. - * * Return: -ENOMEM if there was no room, else 0. */ static int find_slots(struct tcs_group *tcs, const struct tcs_request *msg, @@ -709,25 +704,25 @@ static int find_slots(struct tcs_group *tcs, const struct tcs_request *msg, * This should only be called for for sleep/wake state, never active-only * state. * + * The caller must ensure that no other RPMH actions are happening and the + * controller is idle when this function is called since it runs lockless. + * * Return: 0 if no error; else -error. */ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) { struct tcs_group *tcs; int tcs_id = 0, cmd_id = 0; - unsigned long flags; int ret; tcs = get_tcs_for_msg(drv, msg); if (IS_ERR(tcs)) return PTR_ERR(tcs); - spin_lock_irqsave(&tcs->lock, flags); /* find the TCS id and the command in the TCS to write to */ ret = find_slots(tcs, msg, &tcs_id, &cmd_id); if (!ret) __tcs_buffer_write(drv, tcs_id, cmd_id, msg); - spin_unlock_irqrestore(&tcs->lock, flags); return ret; } @@ -756,8 +751,8 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) * should be checked for not busy, because we used wake TCSes for * active requests in this case. * - * Since this is called from the last cpu, need not take drv or tcs - * lock before checking tcs_is_free(). + * Since this is called from the last cpu, need not take drv->lock + * before checking tcs_is_free(). */ if (!tcs->num_tcs) tcs = &drv->tcs[WAKE_TCS]; @@ -885,7 +880,6 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev, tcs->type = tcs_cfg[i].type; tcs->num_tcs = tcs_cfg[i].n; tcs->ncpt = ncpt; - spin_lock_init(&tcs->lock); if (!tcs->num_tcs || tcs->type == CONTROL_TCS) continue; From patchwork Wed Apr 22 21:55:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11504673 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0252392C for ; Wed, 22 Apr 2020 21:55:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D2D5021D7A for ; Wed, 22 Apr 2020 21:55:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="EIluXLTH" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726377AbgDVVza (ORCPT ); Wed, 22 Apr 2020 17:55:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58572 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726109AbgDVVza (ORCPT ); Wed, 22 Apr 2020 17:55:30 -0400 Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06FF6C03C1A9 for ; Wed, 22 Apr 2020 14:55:30 -0700 (PDT) Received: by mail-pl1-x62c.google.com with SMTP id d24so1476447pll.8 for ; Wed, 22 Apr 2020 14:55:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=KtQzPrZ+vwce/bSxnGi4bn7K6J5NCLu1Fr1QTTaBT78=; b=EIluXLTHtPu3c5CaIMQkXgns139LyT7/kHAMIqCQKWAHxjR1ofPjBSSjrtsJAGnoaA DUtViwaUo1XzLtSamtsfNknRp9n0uIOJIbmEIW2Y0yXUDqKDUqUTVQS6PmUc8VWoi5+C +Fv1GRrInwz3THPc//eT6/KD8z60TuEgzYkak= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=KtQzPrZ+vwce/bSxnGi4bn7K6J5NCLu1Fr1QTTaBT78=; b=czKQBNa9wf94SBGD40CGrIn/dgRPuHBsADssXMDyZRLKFartDE+/AGipOFvvAPVQxQ TP0Kle9Zm6+3QVAmWOIbwiyZXUZJ2jzALlX/Q5uQ6epMGdCesEmPMiYF3H/da0vBQo0s zfDxec9yYefmTKeEdEPeCFgo/yj+1YgQnmokdihKfXYh0u6rSLsYvd6BKVTfk2+RDcQQ Q/+NWQjKnbhDmE6Fv5c52kPZ3GIfK66puYiZ2YjVINnql5SRY5XSi/+oFmVV0ehXbpKF 3tOuqARmRJ1wEOMFYtH/l5C11yUXlPYxaDRek2YKn7oXlnN+a0T09S6+Me7wrdW/L07q s0BQ== X-Gm-Message-State: AGi0PubmpUnVUlhzKRJDrxNXV8Ir1b//VjbzBEMrbf2rMW3u+GGIwCbj U01aA4uioGOkJES0jC316wDp9Q== X-Google-Smtp-Source: APiQypK+MpCj9ipmidWDIOP3u/BEviX6atJDBnZjXe5XOv9yhGrwbLz+AE5XyX5h0cwaB1yFl2H3Eg== X-Received: by 2002:a17:902:6b01:: with SMTP id o1mr802854plk.100.1587592529267; Wed, 22 Apr 2020 14:55:29 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id r4sm211072pgi.6.2020.04.22.14.55.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Apr 2020 14:55:28 -0700 (PDT) From: Douglas Anderson To: Greg Kroah-Hartman , rafael.j.wysocki@intel.com, Andy Gross , Bjorn Andersson Cc: mka@chromium.org, swboyd@chromium.org, mkshah@codeaurora.org, evgreen@chromium.org, Douglas Anderson , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 5/5] soc: qcom: rpmh-rsc: Remove the pm_lock Date: Wed, 22 Apr 2020 14:55:03 -0700 Message-Id: <20200422145408.v4.5.I295cb72bc5334a2af80313cbe97cb5c9dcb1442c@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> References: <20200422145408.v4.1.Ic7096b3b9b7828cdd41cd5469a6dee5eb6abf549@changeid> MIME-Version: 1.0 Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org It has been postulated that the pm_lock is bad for performance because a CPU currently running rpmh_flush() could block other CPUs from coming out of idle. Similarly CPUs coming out of / going into idle all need to contend with each other for the spinlock just to update the variable tracking who's in PM. Let's optimize this a bit. Specifically: - Use a count rather than a bitmask. This is faster to access and also means we can use the atomic_inc_return() function to really detect who the last one to enter PM was. - Accept that it's OK if we race and are doing the flush (because we think we're last) while another CPU is coming out of idle. As long as we block that CPU if/when it tries to do an active-only transfer we're OK. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- Changes in v4: - Rebased atop split-out fixes. Changes in v3: - Rebased atop patch to get rid of per-TCS lock. - Removed bogus comment in rpmh_flush(). - thelock => the lock. - Do one last double-check to try to avoid returning NOTIFY_BAD. Changes in v2: - Always grab drv->lock first to ensure lock ordering. - Grab the cache_lock in rpmh_flush(). - Comments about why num_online_cpus() is OK. - Return NOTIFY_DONE for things we don't care about. - Use trylock to avoid spinning in CPU_PM code. - !rpmh_flush() should have been rpmh_flush(), so we were alwys failing. - Account for CPU_PM_ENTER_FAILED not being called if we return NOTIFY_BAD. drivers/soc/qcom/rpmh-internal.h | 11 +++-- drivers/soc/qcom/rpmh-rsc.c | 72 ++++++++++++++++++++------------ drivers/soc/qcom/rpmh.c | 25 +++++++---- 3 files changed, 66 insertions(+), 42 deletions(-) diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index 1f2857b3f38e..ef60e790a750 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -95,7 +95,7 @@ struct rpmh_ctrlr { * @num_tcs: Number of TCSes in this DRV. * @rsc_pm: CPU PM notifier for controller. * Used when solver mode is not present. - * @cpus_entered_pm: CPU mask for cpus in idle power collapse. + * @cpus_in_pm: Number of CPUs not in idle power collapse. * Used when solver mode is not present. * @tcs: TCS groups. * @tcs_in_use: S/W state of the TCS; only set for ACTIVE_ONLY @@ -103,9 +103,9 @@ struct rpmh_ctrlr { * it was borrowed for an active_only transfer. You * must hold the lock in this struct (AKA drv->lock) in * order to update this. - * @lock: Synchronize state of the controller. - * @pm_lock: Synchronize during PM notifications. - * Used when solver mode is not present. + * @lock: Synchronize state of the controller. If RPMH's cache + * lock will also be held, the order is: drv->lock then + * cache_lock. * @client: Handle to the DRV's client. */ struct rsc_drv { @@ -114,11 +114,10 @@ struct rsc_drv { int id; int num_tcs; struct notifier_block rsc_pm; - struct cpumask cpus_entered_pm; + atomic_t cpus_in_pm; struct tcs_group tcs[TCS_TYPE_NR]; DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR); spinlock_t lock; - spinlock_t pm_lock; struct rpmh_ctrlr client; }; diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 71cebe7fd452..060be10bc491 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -736,6 +736,8 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) * SLEEP and WAKE sets. If AMCs are busy, controller can not enter * power collapse, so deny from the last cpu's pm notification. * + * Context: Must be called with the drv->lock held. + * * Return: * * False - AMCs are idle * * True - AMCs are busy @@ -750,9 +752,6 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) * dedicated TCS for active state use, then re-purposed wake TCSes * should be checked for not busy, because we used wake TCSes for * active requests in this case. - * - * Since this is called from the last cpu, need not take drv->lock - * before checking tcs_is_free(). */ if (!tcs->num_tcs) tcs = &drv->tcs[WAKE_TCS]; @@ -787,42 +786,62 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb, { struct rsc_drv *drv = container_of(nfb, struct rsc_drv, rsc_pm); int ret = NOTIFY_OK; - - spin_lock(&drv->pm_lock); + int cpus_in_pm; switch (action) { case CPU_PM_ENTER: - cpumask_set_cpu(smp_processor_id(), &drv->cpus_entered_pm); - - if (!cpumask_equal(&drv->cpus_entered_pm, cpu_online_mask)) - goto exit; + cpus_in_pm = atomic_inc_return(&drv->cpus_in_pm); + /* + * NOTE: comments for num_online_cpus() point out that it's + * only a snapshot so we need to be careful. It should be OK + * for us to use, though. It's important for us not to miss + * if we're the last CPU going down so it would only be a + * problem if a CPU went offline right after we did the check + * AND that CPU was not idle AND that CPU was the last non-idle + * CPU. That can't happen. CPUs would have to come out of idle + * before the CPU could go offline. + */ + if (cpus_in_pm < num_online_cpus()) + return NOTIFY_OK; break; case CPU_PM_ENTER_FAILED: case CPU_PM_EXIT: - cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); - goto exit; + atomic_dec(&drv->cpus_in_pm); + return NOTIFY_OK; default: return NOTIFY_DONE; } - ret = rpmh_rsc_ctrlr_is_busy(drv); - if (ret) { - ret = NOTIFY_BAD; - goto exit; + /* + * It's likely we're on the last CPU. Grab the drv->lock and write + * out the sleep/wake commands to RPMH hardware. Grabbing the lock + * means that if we race with another CPU coming up we are still + * guaranteed to be safe. If another CPU came up just after we checked + * and has grabbed the lock or started an active transfer then we'll + * notice we're busy and abort. If another CPU comes up after we start + * flushing it will be blocked from starting an active transfer until + * we're done flushing. If another CPU starts an active transfer after + * we release the lock we're still OK because we're no longer the last + * CPU. + */ + if (spin_trylock(&drv->lock)) { + if (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)) + ret = NOTIFY_BAD; + spin_unlock(&drv->lock); + } else { + /* Another CPU must be up */ + return NOTIFY_OK; } - ret = rpmh_flush(&drv->client); - if (ret) - ret = NOTIFY_BAD; - else - ret = NOTIFY_OK; - -exit: - if (ret == NOTIFY_BAD) - /* We won't be called w/ CPU_PM_ENTER_FAILED */ - cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); + if (ret == NOTIFY_BAD) { + /* Double-check if we're here because someone else is up */ + if (cpus_in_pm < num_online_cpus()) + ret = NOTIFY_OK; + else + /* We won't be called w/ CPU_PM_ENTER_FAILED */ + atomic_dec(&drv->cpus_in_pm); + } - spin_unlock(&drv->pm_lock); return ret; } @@ -965,7 +984,6 @@ static int rpmh_rsc_probe(struct platform_device *pdev) solver_config = solver_config >> DRV_HW_SOLVER_SHIFT; if (!solver_config) { drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback; - spin_lock_init(&drv->pm_lock); cpu_pm_register_notifier(&drv->rsc_pm); } diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index d1626a1328d7..f2b5b46ccd1f 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -435,9 +435,6 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state, * * @ctrlr: Controller making request to flush cached data * - * This function is called from sleep code on the last CPU - * (thus no spinlock needed). - * * Return: * * 0 - Success * * Error code - Otherwise @@ -445,13 +442,21 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state, int rpmh_flush(struct rpmh_ctrlr *ctrlr) { struct cache_req *p; - int ret; + int ret = 0; lockdep_assert_irqs_disabled(); + /* + * Currently rpmh_flush() is only called when we think we're running + * on the last processor. If the lock is busy it means another + * processor is up and it's better to abort than spin. + */ + if (!spin_trylock(&ctrlr->cache_lock)) + return -EBUSY; + if (!ctrlr->dirty) { pr_debug("Skipping flush, TCS has latest data.\n"); - return 0; + goto exit; } /* Invalidate the TCSes first to avoid stale data */ @@ -460,7 +465,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr) /* First flush the cached batch requests */ ret = flush_batch(ctrlr); if (ret) - return ret; + goto exit; list_for_each_entry(p, &ctrlr->cache, list) { if (!is_req_valid(p)) { @@ -471,16 +476,18 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr) ret = send_single(ctrlr, RPMH_SLEEP_STATE, p->addr, p->sleep_val); if (ret) - return ret; + goto exit; ret = send_single(ctrlr, RPMH_WAKE_ONLY_STATE, p->addr, p->wake_val); if (ret) - return ret; + goto exit; } ctrlr->dirty = false; - return 0; +exit: + spin_unlock(&ctrlr->cache_lock); + return ret; } /**