<div dir="ltr"><div>yes you are absolutely correct and the change I have done in other area i.e <span style="font-size:11pt;font-family:"Calibri","sans-serif"">drivers/usb/storage/<a href="http://uas.c.In">uas.c.In</a> gpu drivers </span>assert_spin_locked() make<br>
more sense.<br><br></div>Regards<br>Sanjeev Sharma<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 12, 2014 at 12:25 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Aug 11, 2014 at 04:38:50PM -0700, David Rientjes wrote:<br>
> On Mon, 11 Aug 2014, Rob Clark wrote:<br>
><br>
> > > I'm suggesting that if you don't want to incur the cost of the conditional<br>
> > > everytime you call a certain function with assert_spin_locked() that you<br>
> > > could covert these to lockdep_assert_held() so the check is only done when<br>
> > > lockdep is enabled for debugging.<br>
> ><br>
> > not sure about the nouveau parts, but for the omap and msm hunks, this<br>
> > code getting called at potentially vblank rate (so maybe once or twice<br>
> > per ~16ms).. and lockdep has considerable overhead (for a gpu driver)<br>
> > so I don't always have it enabled. So it sounds like for those two at<br>
> > least assert_spin_locked() is a better option.<br>
> ><br>
><br>
> Unless there's a bug, assert_spin_locked() is just going to incur an<br>
> unnecessary cost every time it is called at runtime. My suggestion was to<br>
> limit that check only to debugging kernels that include enabling lockdep<br>
> when tracking down problems rather than needlessly evaluating the<br>
> conditional every time when there are no bugs.<br>
<br>
</div></div>My experience with gpu drivers (i915) has been that hw and the software<br>
running it is varied enough that almost always it's better to<br>
unconditionally enable this stuff. I much prefer assert_spin_locked and<br>
friends over the lockdep versions since enabling full lockdep is not<br>
something you usually do. Especially not normal users, and we rely upon<br>
them for testing the old stuff. Furthermore for the modeset code the<br>
overhead is totally irrelevant since we're doing metric piles of register<br>
reads and writes in there anyway.<br>
<span class="HOEnZb"><font color="#888888">-Daniel<br>
--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="tel:%2B41%20%280%29%2079%20365%2057%2048" value="+41793655748">+41 (0) 79 365 57 48</a> - <a href="http://blog.ffwll.ch" target="_blank">http://blog.ffwll.ch</a><br>
</font></span></blockquote></div><br></div>