[Piglit] [PATCH v3] programs/run.py add file sync command line option
Atwood, Matthew S
matthew.s.atwood at intel.com
Fri Aug 15 09:12:59 PDT 2014
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?
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Friday, August 15, 2014 4:19 AM
To: Atwood, Matthew S
Cc: piglit at lists.freedesktop.org
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 <matthew.s.atwood at intel.com>
>
> 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
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Piglit
mailing list