[Intel-gfx] [PATCH 7/9] drm/i915: allow package C8+ states on Haswell (disabled)

Chris Wilson chris at chris-wilson.co.uk
Sat Aug 10 10:04:45 CEST 2013


On Sat, Aug 10, 2013 at 09:55:14AM +0200, Daniel Vetter wrote:
> On Fri, Aug 9, 2013 at 11:34 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
> > 2013/8/9 Chris Wilson <chris at chris-wilson.co.uk>:
> >> Quick note...
> >>
> >> On Fri, Aug 09, 2013 at 05:10:05PM -0300, Paulo Zanoni wrote:
> >>> +     WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
> >>
> >> Preferred form is now lockdep_assert_held(&dev_priv->pc8.lock);
> >
> > Should I also convert all our other usages of
> > WARN_ON(!mutex_is_locked()) and BUG_ON(!mutex_is_locked()) too? On a
> > separate patch, of course. We have currently no usage of
> > lockdep_assert_held, and I like consistency, so fully switching to the
> > preferred form is good IMHO.
> 
> Tbh I don't understand really why lockdep_assert_held is better ...
> it's right that it also checks that indeed the current task is holding
> the lock (and not some random other imposter). But the downside is
> that it's a noop without CONFIG_PROVE_LOCKING. And due to the massive
> perf impact of that option not many people actually run with it. At
> least I tend to only enable it when doing tricky locking work on my
> dev machines and not in general ...

It doesn't shout so much and people are starting to complain about all
the sanity checks existing outside of the grand lockdep.

Who doesn't have a few machines running with lockdep all the time? ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre



More information about the Intel-gfx mailing list