On Thu, Oct 4, 2012 at 4:39 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > >> 2. w/a patches need testcases, too. Either a register check added to i-g-t >> or if it's a runtime thing, a runtime assert at a nice place (where >> feasible, ofc). > > A register check isn't that useful imo. A real test case would be > ideal, but given how hard some of these issues are to hit, it's > unrealistic to spend weeks writing a test case for a workaround that's > already been documented to fix a specific issue. That's pretty because of Ben's w/a patch to remove the w/a from the part of the init sequence where the write sticks, and keep it at the place where the write doesn't stick. Similarly, we've sometimes managed to apply w/a not correctly after gpu reset or resume. Exactly since for many of these we won't ever have a good test-case, we should at least make sure that we actually succeed in setting the right values everywhere. Shockingly, we've failed even at that :( >> 3. I'll randomly stall patches to bring 2. up to par for existing >> workarounds. > > Btw if you want to take this to its logical conclusion, we also > shouldn't be "fixing" issues that are obvious from code review but > people haven't hit in practice (this goes for a good chunk of the code > churn in our driver involving cleanups and fixes for potential > non-issues). And that's not even including test case development for > any patch claiming it fixes anything. And most of those actually go through dinq, at least if the exact impact is unclear and there's no testcase or bug to demonstrate the issue. My gripes here are purely for pushing too much w/a patches to -fixes, since both your patches and Ben's had regressions that hang machines. Hence my grumpiness. > That said, I definitely agree we want to add more test cases. Just > don't block applying known workaround fixes or other stuff on those > test cases. Sure, I'll usually try to come up with a random distribution not biased against stuff that fixes real bugs ;-) But even for bugfixes I want testcases first (as always, if feasible ofc), since ppl are so easily distracted (and writing testcase is a royal pain). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
