On Thu, 2017-02-09 at 16:57 +0000, Bart Van Assche wrote: > On Thu, 2017-02-09 at 03:48 -0800, Nicholas A. Bellinger wrote: > > As mentioned, these significant of changes require a significant amount > > of focused Q/A regression testing before getting into mainline. > > I haven't seen that happening so far, considering how easy it was to > > break the series with even the most trivial of first order TMR tests. > > Have you really tried to test this code ? I haven't seen any bug report from > you. If you have a test that breaks due to this code please share it. > I spent twenty minutes reviewing -v2 on tuesday, and found a fundamental flaw for the simple order one effect with CMD_T_ABORTED in your approach you've been advocating for the last 12 months. That tells me the interesting code-pathes you've changed where never, ever actually tested in any useful ways until less than 48 hours ago. This tells me you've not thought deeply about, nor tested the more interesting second and third order issues that I've described in detail in the previous review rounds. Which means I've totally lost confidence in your manual testing approach. Then, I spent another 5 hours this morning reviewing every patch line by line, and found all sorts of other issues. Please respond to my concerns inline to each patch, and don't snip and ignore the feedback. > BTW, running the kind of regression tests that is done by a Q/A team is good > but is not a requirement for upstream code. It is much more important that > code is easy to read and to maintain. Uh, no. Significant changes to core functionality require significant regression testing, and review in the full context of the final changes, and not just piecemeal review in a bubble without understanding the second and third order effects the changes cause. What's important for mainline is that functionality that is stable continues to function, and doesn't introduce massive regressions like what you've done for /drivers/target/ in linux-next until I blew the whistle just now. So what would have happened if you managed to sneak all of this unreviewed and untested junk into mainline, and I had not caught the basic flaws to the approach in v4..? You'd had to scramble after the fact to try and fix it on the fly..? We'd end up having to revert like 20 different patches, and it would have been a complete disaster. > > Again, if you have any test that breaks due to these patches please share that > test. So far I haven't seen you post any description of test cases. > I've reviewed all your -v4 patches, and included the ones that I'm comfortable with and have reviews. That is where I'll be leaving it for v4.11-rc1, and we can take it from there. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
