From patchwork Wed Mar 14 08:05:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tvrtko Ursulin X-Patchwork-Id: 10281581 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 5DA4460211 for ; Wed, 14 Mar 2018 08:05:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4EF3928753 for ; Wed, 14 Mar 2018 08:05:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 437D128759; Wed, 14 Mar 2018 08:05:48 +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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 78F7228753 for ; Wed, 14 Mar 2018 08:05:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9E66C6E67B; Wed, 14 Mar 2018 08:05:46 +0000 (UTC) X-Original-To: Intel-gfx@lists.freedesktop.org Delivered-To: Intel-gfx@lists.freedesktop.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0EE616E680 for ; Wed, 14 Mar 2018 08:05:46 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id h76so2186987wme.4 for ; Wed, 14 Mar 2018 01:05:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ursulin-net.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=bSOjeNs+pydvREjFaPv4MGhtFhzwPyOBOxBFmQOzZ3s=; b=ktbRYRdnjef8XOuYA1lfCoVhxEucwsAotIcOW96wo9uRuHfwgxOSqbdog6GqeJz3vd W8BHCoh/uzLYnWaiXfZkBC6iOPjhWoPYTi4byyW1sSVV7aT4h6aXWrjlMHBVw1nP2KGr pTK5JFRJsu+buPC5D+bMyYaPKJZWcq+5o0QOEzqc3e5Jl9v3Xhk1ugippLsroU0cdnZm +VX7tfy59/mezZLLJifdpbNpqR8G403vv8Fz8OJQrz1Imy510e6NnJ4DDoK/OGwhi87u iOFfm7R7YzA/UOt98Qk8CoWRRc6y60CZVcTbgBfBP6txsvvpD7nRQ0CZnKuWXfk2rsht jMcw== 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=bSOjeNs+pydvREjFaPv4MGhtFhzwPyOBOxBFmQOzZ3s=; b=riat0WyMhddsd1upyDbYhPFwwgkeaiz2RXiD/0i3wpx5fnl5a+Tyiyoc4Fko09MSTq cX1t8eypF+SavWgz6SLRtTIeQvctUhBU1MT/tHrokjVgS+oRn9dvcv/pHkZbLY2eDeMI Eq1JthwViM+3yoESROMuMCyEHj2mDPBQ1BmbXXh67+4RUhhQSEnY9bXxQXagnj99dTLk TBDv/Uh5mqukTz5jMLZBBN5Av2Tw5Y3ttd7L75qoRgM46t0N5b0CDpALv1i2xusXOtp7 1A/ZQc4wkGdA2bZmNNjB1QjeYQ3C7tC47Ht+iYXaUK5VCRPWWpteEWlkLcCKc1RPY+M1 mSGA== X-Gm-Message-State: AElRT7GGlhDCHmmnRUGYb/Yc5yG76Tbi+RE/8XfiYhBjRizHQX55T3H9 7t4L6JyWe7YnWQYo0yomqbwcNq0P X-Google-Smtp-Source: AG47ELvOAEV9Z/QgXEqt2VL/Ri41QtkleLeTd0H+5FVGxJMJ/wMGL8RF28vuOpZfzHMqmyqu1zvdyg== X-Received: by 10.28.146.68 with SMTP id u65mr879786wmd.107.1521014744504; Wed, 14 Mar 2018 01:05:44 -0700 (PDT) Received: from localhost.localdomain ([95.146.144.186]) by smtp.gmail.com with ESMTPSA id 31sm1498723wrr.59.2018.03.14.01.05.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Mar 2018 01:05:44 -0700 (PDT) From: Tvrtko Ursulin X-Google-Original-From: Tvrtko Ursulin To: Intel-gfx@lists.freedesktop.org Date: Wed, 14 Mar 2018 08:05:35 +0000 Message-Id: <20180314080535.17490-1-tvrtko.ursulin@linux.intel.com> X-Mailer: git-send-email 2.14.1 Subject: [Intel-gfx] [PATCH] drm/i915/pmu: Work around compiler warnings on some kernel configs X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , dri-devel@lists.freedesktop.org, Rodrigo Vivi , intel-gfx@lists.freedesktop.org MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Tvrtko Ursulin Arnd Bergman reports: """ The conditional spinlock confuses gcc into thinking the 'flags' value might contain uninitialized data: drivers/gpu/drm/i915/i915_pmu.c: In function '__i915_pmu_event_read': arch/x86/include/asm/paravirt_types.h:573:3: error: 'flags' may be used uninitialized in this function [-Werror=maybe-uninitialized] The code is correct, but it's easy to see how the compiler gets confused here. This avoids the problem by pulling the lock outside of the function into its only caller. """ On deeper look it seems this is caused by paravirt spinlocks implementation when CONFIG_PARAVIRT_DEBUG is set, which by being complicated, manages to convince gcc locked parameter can be changed externally (impossible). Work around it by removing the conditional locking parameters altogether. (It was never the most elegant code anyway.) Slight penalty we now pay is an additional irqsave spin lock/unlock cycle on the event enable path. But since enable is not a fast path, that is preferrable to the alternative solution which was doing MMIO under irqsave spinlock. Signed-off-by: Tvrtko Ursulin Reported-by: Arnd Bergmann Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Cc: Tvrtko Ursulin Cc: Chris Wilson Cc: Imre Deak Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: David Airlie Cc: intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Reviewed-by: Chris Wilson Acked-by: Arnd Bergmann --- drivers/gpu/drm/i915/i915_pmu.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 4bc7aefa9541..11fb76bd3860 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -412,7 +412,7 @@ static u64 __get_rc6(struct drm_i915_private *i915) return val; } -static u64 get_rc6(struct drm_i915_private *i915, bool locked) +static u64 get_rc6(struct drm_i915_private *i915) { #if IS_ENABLED(CONFIG_PM) unsigned long flags; @@ -428,8 +428,7 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) * previously. */ - if (!locked) - spin_lock_irqsave(&i915->pmu.lock, flags); + spin_lock_irqsave(&i915->pmu.lock, flags); if (val >= i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) { i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = 0; @@ -438,12 +437,10 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur; } - if (!locked) - spin_unlock_irqrestore(&i915->pmu.lock, flags); + spin_unlock_irqrestore(&i915->pmu.lock, flags); } else { struct pci_dev *pdev = i915->drm.pdev; struct device *kdev = &pdev->dev; - unsigned long flags2; /* * We are runtime suspended. @@ -452,10 +449,8 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) * on top of the last known real value, as the approximated RC6 * counter value. */ - if (!locked) - spin_lock_irqsave(&i915->pmu.lock, flags); - - spin_lock_irqsave(&kdev->power.lock, flags2); + spin_lock_irqsave(&i915->pmu.lock, flags); + spin_lock(&kdev->power.lock); if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) i915->pmu.suspended_jiffies_last = @@ -465,14 +460,13 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) i915->pmu.suspended_jiffies_last; val += jiffies - kdev->power.accounting_timestamp; - spin_unlock_irqrestore(&kdev->power.lock, flags2); + spin_unlock(&kdev->power.lock); val = jiffies_to_nsecs(val); val += i915->pmu.sample[__I915_SAMPLE_RC6].cur; i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val; - if (!locked) - spin_unlock_irqrestore(&i915->pmu.lock, flags); + spin_unlock_irqrestore(&i915->pmu.lock, flags); } return val; @@ -481,7 +475,7 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) #endif } -static u64 __i915_pmu_event_read(struct perf_event *event, bool locked) +static u64 __i915_pmu_event_read(struct perf_event *event) { struct drm_i915_private *i915 = container_of(event->pmu, typeof(*i915), pmu.base); @@ -519,7 +513,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event, bool locked) val = count_interrupts(i915); break; case I915_PMU_RC6_RESIDENCY: - val = get_rc6(i915, locked); + val = get_rc6(i915); break; } } @@ -534,7 +528,7 @@ static void i915_pmu_event_read(struct perf_event *event) again: prev = local64_read(&hwc->prev_count); - new = __i915_pmu_event_read(event, false); + new = __i915_pmu_event_read(event); if (local64_cmpxchg(&hwc->prev_count, prev, new) != prev) goto again; @@ -584,14 +578,14 @@ static void i915_pmu_enable(struct perf_event *event) engine->pmu.enable_count[sample]++; } + spin_unlock_irqrestore(&i915->pmu.lock, flags); + /* * Store the current counter value so we can report the correct delta * for all listeners. Even when the event was already enabled and has * an existing non-zero value. */ - local64_set(&event->hw.prev_count, __i915_pmu_event_read(event, true)); - - spin_unlock_irqrestore(&i915->pmu.lock, flags); + local64_set(&event->hw.prev_count, __i915_pmu_event_read(event)); } static void i915_pmu_disable(struct perf_event *event)