Re: [igt-dev] [PATCH i-g-t 12/17] benchmarks/gem_wsim: extract allocate and prepare contexts code to new functions

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

On 28/09/2023 18:45, Marcin Bernatowicz wrote:
> No functional changes.
> Extracted allocate_contexts and prepare_contexts functions
> from prepare_workload.
> 
> v2:
> - propagate error code from prepare_contexts (Tvrtko)
> - don't mix unrelated changes (Tvrtko)
> 
> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
> ---
>   benchmarks/gem_wsim.c | 43 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
> index 7495ab297..a344fea9e 100644
> --- a/benchmarks/gem_wsim.c
> +++ b/benchmarks/gem_wsim.c
> @@ -1764,19 +1764,11 @@ static void measure_active_set(struct workload *wrk)
>   
>   #define alloca0(sz) ({ size_t sz__ = (sz); memset(alloca(sz__), 0, sz__); })
>   
> -static int prepare_workload(unsigned int id, struct workload *wrk)
> +static void allocate_contexts(unsigned int id, struct workload *wrk)
>   {
> -	struct working_set **sets;
> -	unsigned long total = 0;
> -	uint32_t share_vm = 0;
>   	int max_ctx = -1;
>   	struct w_step *w;
> -	int i, j;
> -
> -	wrk->id = id;
> -	wrk->bb_prng = (wrk->flags & FLAG_SYNCEDCLIENTS) ? master_prng : rand();
> -	wrk->bo_prng = (wrk->flags & FLAG_SYNCEDCLIENTS) ? master_prng : rand();
> -	wrk->run = true;
> +	int i;
>   
>   	/*
>   	 * Pre-scan workload steps to allocate context list storage.
> @@ -1800,6 +1792,13 @@ static int prepare_workload(unsigned int id, struct workload *wrk)
>   
>   		max_ctx = ctx;
>   	}
> +}
> +
> +static int prepare_contexts(unsigned int id, struct workload *wrk)
> +{
> +	uint32_t share_vm = 0;
> +	struct w_step *w;
> +	int i, j;
>   
>   	/*
>   	 * Transfer over engine map configuration from the workload step.
> @@ -1966,6 +1965,28 @@ static int prepare_workload(unsigned int id, struct workload *wrk)
>   	if (share_vm)
>   		vm_destroy(fd, share_vm);
>   
> +	return 0;
> +}
> +
> +static int prepare_workload(unsigned int id, struct workload *wrk)
> +{
> +	struct working_set **sets;
> +	unsigned long total = 0;
> +	struct w_step *w;
> +	int i, j;
> +	int ret = 0;
> +
> +	wrk->id = id;
> +	wrk->bb_prng = (wrk->flags & FLAG_SYNCEDCLIENTS) ? master_prng : rand();
> +	wrk->bo_prng = (wrk->flags & FLAG_SYNCEDCLIENTS) ? master_prng : rand();
> +	wrk->run = true;
> +
> +	allocate_contexts(id, wrk);
> +
> +	ret = prepare_contexts(id, wrk);
> +	if (ret)
> +		return ret;
> +
>   	/* Record default preemption. */
>   	for (i = 0, w = wrk->steps; i < wrk->nr_steps; i++, w++) {
>   		if (w->type == BATCH)
> @@ -2067,7 +2088,7 @@ static int prepare_workload(unsigned int id, struct workload *wrk)
>   
>   	measure_active_set(wrk);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static double elapsed(const struct timespec *start, const struct timespec *end)

Looks fine.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko