Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] benchmarks/gem_wsim: Randomise random seed

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/05/2019 09:37, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-17 09:28:46)
>>
>> On 16/05/2019 19:58, Chris Wilson wrote:
>>> To avoid hitting the same rut on each benchmark run, start with a new
>>> random seed. To allow hitting the same rut again, let it be specified
>>> by the user.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>    benchmarks/gem_wsim.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>>> index 48568ce40..cf2a44746 100644
>>> --- a/benchmarks/gem_wsim.c
>>> +++ b/benchmarks/gem_wsim.c
>>> @@ -2282,8 +2282,9 @@ int main(int argc, char **argv)
>>>        igt_require(fd);
>>>    
>>>        init_clocks();
>>> +     srand(time(NULL));
>>>    
>>> -     while ((c = getopt(argc, argv, "hqv2RSHxGdc:n:r:w:W:a:t:b:p:")) != -1) {
>>> +     while ((c = getopt(argc, argv, "hqv2RSHxGdc:n:r:w:W:a:t:b:p:s:")) != -1) {
>>>                switch (c) {
>>>                case 'W':
>>>                        if (master_workload >= 0) {
>>> @@ -2300,6 +2301,9 @@ int main(int argc, char **argv)
>>>                case 'p':
>>>                        prio = atoi(optarg);
>>>                        break;
>>> +             case 's':
>>> +                     srand(atoi(optarg));
>>> +                     break;
>>>                case 'a':
>>>                        if (append_workload_arg) {
>>>                                if (verbose)
>>>
>>
>> Makes sense, just the help text missing.
> 
> You use 's' for sseu, I think I (Initial random seed) is one of the few
> available.

No complaints on 'I'.

>> I was initially confused at srand when I thought I used your
>> hars_petruska library routines but then saw rand() feeds the seed into
>> those.
> 
> Yeah, I had to go and double check that we actually used rand()
> somewhere (and not just in my hacks :)

Seems I have to separate streams of randomness.

One is relating to random mode load balancing, which this patch influences.

Second is relating to batch buffer duration which is driven from the 
global hars_petruska_f54_1_random_seed and wrongly uses the _unsafe 
function from unlocked section. This might be from before I replaced 
forking with threads. :(

I think the aggregate fix would be to store a second seed per workload 
for the latter. Replace hars_pretruska_f54_1_random_seed near beginning 
of run_worklaod with wrk->bb_prng = (wrk->flags & SYNCEDCLIENTS) ? 
master_prng_seed : time(NULL); And in get_duration use wrk->bb_prng and 
safe pnrg function. (You store master_prng_seed in main after it is set.)

I think this should fix all issues and propagate the passed in seed 
correctly.


Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev