On 15 August 2014 17:12, Atwood, Matthew S <[email protected]> wrote: > Ah, I was wondering what the proper way to do that was, sorry for the > confusion. My go to was to respond to previous version messages as whether or > not the feedback had been added. As far as default behavior for igt tests, I > wouldn't be averse to it, maybe Thomas Wood can chime in on that? >
As long as this doesn't add a significant overhead when running all the tests, I think it would be a good idea to enable it by default for igt.py. > -----Original Message----- > From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter > Sent: Friday, August 15, 2014 4:19 AM > To: Atwood, Matthew S > Cc: [email protected] > Subject: Re: [Piglit] [PATCH v3] programs/run.py add file sync command line > option > > On Tue, Jan 03, 2012 at 09:08:25PM -0800, Matthew Atwood wrote: >> From: Matt Atwood <[email protected]> >> >> Currently while running igt tests a kernel panic causes the results >> json file to lose all data. This patch adds a command line option (-s, >> --sync) that syncs the file descriptor to disk after every test >> allowing data to persist through hard crashes. > > When resending patches please add a short changelog here (or below the --- > line, not sure about piglit standard) so that people can see at a glance what > changed. > > I wonder whether we shouldn't set this by default in tests/igt.py like we do > with dmesg already. igt testcases are a lot slower than opengl piglit tests, > so the added overhead should hurt badly. And if it does we can always add a > --no-sync later on. > -Daniel >> --- >> framework/core.py | 3 ++- >> framework/programs/run.py | 14 ++++++++++---- >> framework/results.py | 15 ++++++++++++++- >> 3 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/framework/core.py b/framework/core.py index >> d3922a9..8c4ff57 100644 >> --- a/framework/core.py >> +++ b/framework/core.py >> @@ -100,7 +100,7 @@ class Options(object): >> """ >> def __init__(self, concurrent=True, execute=True, include_filter=None, >> exclude_filter=None, valgrind=False, dmesg=False, >> - verbose=False): >> + verbose=False, sync=False): >> self.concurrent = concurrent >> self.execute = execute >> self.filter = [re.compile(x) for x in include_filter or []] >> @@ -109,6 +109,7 @@ class Options(object): >> self.valgrind = valgrind >> self.dmesg = dmesg >> self.verbose = verbose >> + self.sync = sync >> # env is used to set some base environment variables that are not >> going >> # to change across runs, without sending them to os.environ which is >> # fickle as easy to break >> diff --git a/framework/programs/run.py b/framework/programs/run.py >> index eb67d7f..468bff4 100644 >> --- a/framework/programs/run.py >> +++ b/framework/programs/run.py >> @@ -97,6 +97,9 @@ def run(input_): >> type=path.realpath, >> metavar="<Results Path>", >> help="Path to results folder") >> + parser.add_argument("-s", "--sync", >> + action="store_true", >> + help="Sync results to disk after every test") >> args = parser.parse_args(input_) >> >> # Disable Windows error message boxes for this and all child processes. >> @@ -132,7 +135,8 @@ def run(input_): >> execute=args.execute, >> valgrind=args.valgrind, >> dmesg=args.dmesg, >> - verbose=args.verbose) >> + verbose=args.verbose, >> + sync=args.sync) >> >> # Set the platform to pass to waffle >> if args.platform: >> @@ -154,7 +158,7 @@ def run(input_): >> # Begin json. >> result_filepath = path.join(args.results_path, 'results.json') >> result_file = open(result_filepath, 'w') >> - json_writer = framework.results.JSONWriter(result_file) >> + json_writer = framework.results.JSONWriter(result_file, >> + opts.sync) >> >> # Create a dictionary to pass to initialize json, it needs the contents >> of >> # the env dictionary and profile and platform information @@ >> -211,7 +215,8 @@ def resume(input_): >> execute=results.options['execute'], >> valgrind=results.options['valgrind'], >> dmesg=results.options['dmesg'], >> - verbose=results.options['verbose']) >> + verbose=results.options['verbose'], >> + sync=results.options['sync']) >> >> core.get_config(args.config_file) >> >> @@ -219,7 +224,8 @@ def resume(input_): >> opts.env['PIGLIT_PLATFORM'] = results.options['platform'] >> >> results_path = path.join(args.results_path, 'results.json') >> - json_writer = framework.results.JSONWriter(open(results_path, 'w+')) >> + json_writer = framework.results.JSONWriter(open(results_path, 'w+'), >> + opts.sync) >> json_writer.initialize_json(results.options, results.name, >> core.collect_system_info()) >> >> diff --git a/framework/results.py b/framework/results.py index >> 4b5fb30..efc7029 100644 >> --- a/framework/results.py >> +++ b/framework/results.py >> @@ -102,8 +102,9 @@ class JSONWriter(object): >> >> INDENT = 4 >> >> - def __init__(self, f): >> + def __init__(self, f, fsync): >> self.file = f >> + self.fsync = fsync >> self.__indent_level = 0 >> self.__inhibit_next_indent = False >> self.__encoder = json.JSONEncoder(indent=self.INDENT, >> @@ -175,6 +176,12 @@ class JSONWriter(object): >> self.file.close() >> >> @synchronized_self >> + def __file_sync(self): >> + if self.fsync: >> + self.file.flush() >> + os.fsync(self.file.fileno()) >> + >> + @synchronized_self >> def __write_indent(self): >> if self.__inhibit_next_indent: >> self.__inhibit_next_indent = False @@ -201,6 +208,7 @@ >> class JSONWriter(object): >> self.__indent_level += 1 >> self.__is_collection_empty.append(True) >> self._open_containers.append('dict') >> + self.__file_sync() >> >> @synchronized_self >> def close_dict(self): >> @@ -212,6 +220,7 @@ class JSONWriter(object): >> self.file.write('}') >> assert self._open_containers[-1] == 'dict' >> self._open_containers.pop() >> + self.__file_sync() >> >> @synchronized_self >> def write_dict_item(self, key, value): >> @@ -221,6 +230,8 @@ class JSONWriter(object): >> # Write value. >> self.__write(value) >> >> + self.__file_sync() >> + >> @synchronized_self >> def write_dict_key(self, key): >> # Write comma if this is not the initial item in the dict. >> @@ -235,6 +246,8 @@ class JSONWriter(object): >> >> self.__inhibit_next_indent = True >> >> + self.__file_sync() >> + >> >> class TestResult(dict): >> def __init__(self, *args): >> -- >> 1.8.3.2 >> >> _______________________________________________ >> Piglit mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/piglit > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Piglit mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
