[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Luck, Tony]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

> If shared wq is not supported by platform use dedicated mode to avoid
> failing batch configuration tests

Seems weird to change type of WQ. If the caller asked for a shared WQ, they are expecting
to send work to it using ENQCMD or ENQCMDS. Switching to dedicated would make the
requester still fail ... unless they subsequently check to see that this type switch happened.

-Tony

[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

On Wed, Aug 11, 2021 at 11:11:59AM, Tony Luck wrote:
> > This is the unit test that only tests the configuration. It configures a
> > few wqs as shared and dedicated. The option is to skip the wqs set to be
> > tested as shared wqs or configure all wqs as dedicated. There is no
> > change to the config-wq command. I can add "test" to the tag to avoid
> > confusion.
> 
> Sounds like the confusion begins with the specification for the test.
> 
> Perhaps divide into two parts:
> 1) Creation of dedicated WQs
> Possible results are PASS or FAIL
> 2) Creation of shared WQs
> Possible results are PASS, FAIL or SKIP

That's a good idea. I can split it into 2 tests for shared and dedicated.

> 
> -Tony

[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Luck, Tony]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

> This is the unit test that only tests the configuration. It configures a
> few wqs as shared and dedicated. The option is to skip the wqs set to be
> tested as shared wqs or configure all wqs as dedicated. There is no
> change to the config-wq command. I can add "test" to the tag to avoid
> confusion.

Sounds like the confusion begins with the specification for the test.

Perhaps divide into two parts:
1) Creation of dedicated WQs
	Possible results are PASS or FAIL
2) Creation of shared WQs
	Possible results are PASS, FAIL or SKIP

-Tony

[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On Wed, Aug 11, 2021 at 10:08:28AM, Tony Luck wrote:
> > If shared wq is not supported by platform use dedicated mode to avoid
> > failing batch configuration tests
> 
> Seems weird to change type of WQ. If the caller asked for a shared WQ, they are expecting
> to send work to it using ENQCMD or ENQCMDS. Switching to dedicated would make the
> requester still fail ... unless they subsequently check to see that this type switch
> happened.

This is the unit test that only tests the configuration. It configures a
few wqs as shared and dedicated. The option is to skip the wqs set to be
tested as shared wqs or configure all wqs as dedicated. There is no
change to the config-wq command. I can add "test" to the tag to avoid
confusion.


> 
> -Tony

[Accel-config] Re: [PATCH] accel-config: Do not fail shared wq tests if platform does not support shared wqs

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]


On 8/10/2021 7:55 PM, ramesh.thomas(a)intel.com wrote:
> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
>
> If shared wq is not supported by platform use dedicated mode to avoid
> failing batch configuration tests
>
> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>

Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>


> ---
>   test/libaccfg.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/test/libaccfg.c b/test/libaccfg.c
> index 516adbc..a982344 100644
> --- a/test/libaccfg.c
> +++ b/test/libaccfg.c
> @@ -244,7 +244,13 @@ static int config_wq(struct accfg_ctx *ctx, struct accfg_device *device,
>   {
>   	int rc = 0;
>   
> -	SET_ERR(rc, accfg_wq_set_str_mode(wq, wq_param->mode));
> +	rc = accfg_wq_set_str_mode(wq, wq_param->mode);
> +	if (rc == -EINVAL) {
> +		fprintf(stderr, "%s shared wq not supported, configuring as dedicated\n",
> +				__func__);
> +		rc = accfg_wq_set_str_mode(wq, "dedicated");
> +	}
> +
>   	SET_ERR(rc, accfg_wq_set_str_type(wq, wq_param->type));
>   	SET_ERR(rc, accfg_wq_set_str_name(wq, wq_param->name));
>   	SET_ERR(rc, accfg_wq_set_size(wq, wq_param->wq_size));
> @@ -254,7 +260,7 @@ static int config_wq(struct accfg_ctx *ctx, struct accfg_device *device,
>   	SET_ERR(rc, accfg_wq_set_max_batch_size(wq, wq_param->max_batch_size));
>   	SET_ERR(rc, accfg_wq_set_max_transfer_size(wq,
>   				wq_param->max_transfer_size));
> -	if (wq_param->threshold)
> +	if (wq_param->threshold && accfg_wq_get_mode(wq) == ACCFG_WQ_SHARED)
>   		SET_ERR(rc, accfg_wq_set_threshold(wq, wq_param->threshold));
>   
>   	SET_ERR(rc, accfg_wq_set_ats_disable(wq, wq_param->ats_disable));
> @@ -288,7 +294,7 @@ static int check_wq(struct accfg_ctx *ctx, struct accfg_device *device,
>   		fprintf(stderr, "%s failed on block_on_fault\n", __func__);
>   		return -EINVAL;
>   	}
> -	if (wq_param->threshold !=
> +	if (accfg_wq_get_mode(wq) == ACCFG_WQ_SHARED && wq_param->threshold !=
>   			(unsigned int)accfg_wq_get_threshold(wq)) {
>   		fprintf(stderr, "%s failed on threshold\n", __func__);
>   		return -EINVAL;
> @@ -815,6 +821,11 @@ static int test_mdev_1swq(struct accfg_ctx *ctx)
>   	if (rc)
>   		return rc;
>   
> +	if (accfg_wq_get_mode(test_ctx.wq[2]) != ACCFG_WQ_SHARED) {
> +		fprintf(stderr, "%s shared wq not supported\n", __func__);
> +		return -EOPNOTSUPP;
> +	}
> +
>   	rc = set_mdev_type(ctx, test_ctx.wq[2], test_ctx.wq_param[2]);
>   	if (rc)
>   		return rc;