Messages in this thread | | | Subject | Re: [Intel-gfx] [PATCH] [v2] drm/i915/pmu: avoid -Wmaybe-uninitialized warning | From | Tvrtko Ursulin <> | Date | Tue, 13 Mar 2018 17:46:54 +0000 |
| |
On 13/03/2018 16:19, Arnd Bergmann wrote: > 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]
Hm, how does paravirt_types.h comes into the picture?
> 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.
Is it specific gcc version, specific options, or specific kernel config that this happens?
Strange that it hasn't been seen so far.
Regards,
Tvrtko
> Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > v2: removed unused function argument, fixed 'break' statement. > --- > drivers/gpu/drm/i915/i915_pmu.c | 25 ++++++++++--------------- > 1 file changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 4bc7aefa9541..d6b9b6b5fb98 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -412,10 +412,9 @@ 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; > u64 val; > > if (intel_runtime_pm_get_if_in_use(i915)) { > @@ -428,18 +427,12 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) > * previously. > */ > > - if (!locked) > - 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; > i915->pmu.sample[__I915_SAMPLE_RC6].cur = val; > } else { > val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur; > } > - > - if (!locked) > - spin_unlock_irqrestore(&i915->pmu.lock, flags); > } else { > struct pci_dev *pdev = i915->drm.pdev; > struct device *kdev = &pdev->dev; > @@ -452,9 +445,6 @@ 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); > > if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) > @@ -470,9 +460,6 @@ static u64 get_rc6(struct drm_i915_private *i915, bool locked) > 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); > } > > return val; > @@ -519,7 +506,15 @@ 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); > + if (!locked) { > + unsigned long flags; > + > + spin_lock_irqsave(&i915->pmu.lock, flags); > + val = get_rc6(i915); > + spin_unlock_irqrestore(&i915->pmu.lock, flags); > + } else { > + val = get_rc6(i915); > + } > break; > } > } >
| |