On Mon, Nov 23, 2015 at 08:54:46PM -0500, Ilia Mirkin wrote: > On Mon, Nov 23, 2015 at 8:41 PM, Dylan Baker <baker.dyla...@gmail.com> wrote: > > On Mon, Nov 23, 2015 at 08:24:22PM -0500, Ilia Mirkin wrote: > >> On Mon, Nov 23, 2015 at 8:07 PM, Dylan Baker <baker.dyla...@gmail.com> > >> wrote: > >> > On Mon, Nov 23, 2015 at 02:48:35PM +0200, Martin Peres wrote: > >> >> On 02/11/15 16:57, yann.argo...@linux.intel.com wrote: > >> >> >This adds a policy which advises when user should reboot system to > >> >> >avoid > >> >> >noisy test results due to system becoming unstable, for instance, and > >> >> >therefore continues testing successfully. > >> >> >To do this, a new Dmesg class is proposed which is not filtering dmesg > >> >> >and > >> >> >monitors whether or not one of the following event occurs: > >> >> >- gpu reset failed (not just gpu reset happened, that happens way too > >> >> >often and many tests even provoke hangs intentionally) > >> >> >- gpu crash, > >> >> >- Oops: > >> >> >- BUG > >> >> >- lockdep splat that causes the locking validator to get disabled > >> >> >If one of these issues happen, piglit test execution is stopped > >> >> >-terminating test thread pool- and exit with code 3 to inform that > >> >> >reboot > >> >> >is advised. > >> >> >Then test execution resume, after rebooting system or not, is done like > >> >> >usually with command line parameter "resume". > >> >> > > >> >> >To call it, use command line parameter: --dmesg monitored > >> >> > >> >> Hello Yann, > >> >> > >> >> The rationale behind this patch is very sound and we need something like > >> >> this. Here are however a list of nitpicks: > >> >> > >> >> - Please send patches with git send-email, otherwise, it makes it > >> >> impossible for us to comment inline which is the usual process for patch > >> >> review. Please re-send :) > >> >> > >> >> - varaiable -> variable; double space after "when a reboot may be > >> >> required" > >> >> > >> >> - I am not a big fan of changing the semantic of arguments that have > >> >> been > >> >> there forever. Can you think of a case where the user would not want the > >> >> test to abort if we reach a state where we cannot trust the result? I am > >> >> including Dylan on this. Also, if we are to keep these modes, can we > >> >> rename > >> >> the "simple" mode to "warning" and "monitored" to "abort"? This would > >> >> make > >> >> more sense and clearly state the goal of the modes. > >> > > >> > Ilia, Daniel, Thomas, Glenn, I know that y'all use the dmesg support. > >> > What do you think? > >> > >> Can you provide a summary of what this patch does? It was submitted as > >> an attachment, so I can't (easily) look at it... either way, as long > >> as running with --dmesg doesn't break, I probably don't care. I use > >> --dmesg to know which tests cause the GPU to complain, which I then, > >> in turn, use to pick which tests to debug further. (And since I > >> normally run with -1 anyways, it's ~free to add...) > >> > >> -ilia > > > > I'm going to interpret that as "I'd be annoyed if piglit just stopped > > when there was dmesg chatter"? > > That would definitely be a deal breaker. My wifi goes in and out and > complains loudly about it in dmesg :) > > -ilia
I think that answers that question Martin. I would lean towards adding a "--dmesg-abort" (or whatever name) flag to be passed in addition to --dmesg when the user wants to stop on dmesg errors. However, I'd really like to see the patch come in with git-send-email so I can have a proper look at it. Dylan
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
