* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 2:31 Zhu, Tony
0 siblings, 0 replies; 6+ messages in thread
From: Zhu, Tony @ 2022-04-28 2:31 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]
It is used in other functions, could not be removed.
Tony(zhu, xinzhan)
Cube:SHZ1-3W-279
iNet:8821-5077
-----Original Message-----
From: Jiang, Dave <dave.jiang(a)intel.com>
Sent: Thursday, April 28, 2022 10:20 AM
To: Zhu, Tony <tony.zhu(a)intel.com>; accel-config(a)lists.01.org
Cc: Thomas, Ramesh <ramesh.thomas(a)intel.com>; Buchanan, Kenneth <kenneth.buchanan(a)intel.com>
Subject: Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
On 4/27/2022 7:15 PM, Zhu, Tony wrote:
> Dave,
>
> The code base is pending branch. It included the common code refactoring.
>
> I've tested it. msec_timeout is defined in the function.
>
> int acctest_wait_on_desc_timeout(struct completion_record *comp,
> struct acctest_context *ctx,
> unsigned int msec_timeout)
>
> unsigned int ms_timeout = 5000, should not use it in this function.
Gotcha. Should the unused var be removed? Otherwise Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>
>
> Tony(zhu, xinzhan)
> Cube:SHZ1-3W-279
> iNet:8821-5077
>
> -----Original Message-----
> From: Jiang, Dave <dave.jiang(a)intel.com>
> Sent: Thursday, April 28, 2022 9:49 AM
> To: Zhu, Tony <tony.zhu(a)intel.com>; accel-config(a)lists.01.org
> Cc: Thomas, Ramesh <ramesh.thomas(a)intel.com>; Buchanan, Kenneth
> <kenneth.buchanan(a)intel.com>
> Subject: Re: [PATCH v1 1/1] accel-config/test: Fix description timeout
> calculation to overflow
>
>
> On 4/27/2022 6:31 PM, Tony Zhu wrote:
>> Dsa_test application will fail when when utilizing memory sizes
>> larger than 128MiB against multiple devices. The init of timeout
>> should use the input msec_timeout.
>>
>> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
> Ken, you want your sign off added?
>
>> ---
>> test/accel_test.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/accel_test.c b/test/accel_test.c index
>> 8487099..0468a00 100644
>> --- a/test/accel_test.c
>> +++ b/test/accel_test.c
>> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct completion_record *comp,
>> j++;
>> }
>> } else {
>> - unsigned long timeout = (ms_timeout * 1000000) * 3;
>> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
> Couple things. This looks like changed with the common code refactoring?
> Has that merged by Ramesh? If not then maybe do the diff before that code since we need to get this fix released. And then make sure the refactoring take this change into account.
>
> Also, did you compile test this? I don't see the var name ms_timeout changed to msec_timeout.
>
>> int r = 1;
>> unsigned long t = 0;
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 15:39 Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-04-28 15:39 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]
On 4/28/2022 8:32 AM, Dave Jiang wrote:
>
> On 4/27/2022 6:31 PM, Tony Zhu wrote:
>> Dsa_test application will fail when when utilizing memory sizes larger
>> than 128MiB against multiple devices. The init of timeout should use
>> the input msec_timeout.
The commit message should say:
Testing has shown that when using large transfer size, the mwait value
is not working correctly. The dsa_test application will fail due to
constant value converted by compiler to integer and cause the math to
overflow on large values. Explicitly declare constant to UL in order to
force type casting of unsigned long. Also, the calculation should use
the passed in timeout parameter and not the global constant.
Tony, can you also add a patch and remove the global variable ms_timeout
since it's not being used anywhere now? This is not urgent. Thanks!
> Signed-off-by: Ken Buchanan <kenneth.buchanan(a)intel.com>
>> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
>
> Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>
>
>
>> ---
>> test/accel_test.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/accel_test.c b/test/accel_test.c
>> index 8487099..0468a00 100644
>> --- a/test/accel_test.c
>> +++ b/test/accel_test.c
>> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct
>> completion_record *comp,
>> j++;
>> }
>> } else {
>> - unsigned long timeout = (ms_timeout * 1000000) * 3;
>> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
>> int r = 1;
>> unsigned long t = 0;
> _______________________________________________
> Accel-config mailing list -- accel-config(a)lists.01.org
> To unsubscribe send an email to accel-config-leave(a)lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 15:32 Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-04-28 15:32 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
On 4/27/2022 6:31 PM, Tony Zhu wrote:
> Dsa_test application will fail when when utilizing memory sizes larger
> than 128MiB against multiple devices. The init of timeout should use
> the input msec_timeout.
Signed-off-by: Ken Buchanan <kenneth.buchanan(a)intel.com>
> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>
> ---
> test/accel_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/accel_test.c b/test/accel_test.c
> index 8487099..0468a00 100644
> --- a/test/accel_test.c
> +++ b/test/accel_test.c
> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct completion_record *comp,
> j++;
> }
> } else {
> - unsigned long timeout = (ms_timeout * 1000000) * 3;
> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
> int r = 1;
> unsigned long t = 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 2:19 Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-04-28 2:19 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]
On 4/27/2022 7:15 PM, Zhu, Tony wrote:
> Dave,
>
> The code base is pending branch. It included the common code refactoring.
>
> I've tested it. msec_timeout is defined in the function.
>
> int acctest_wait_on_desc_timeout(struct completion_record *comp,
> struct acctest_context *ctx,
> unsigned int msec_timeout)
>
> unsigned int ms_timeout = 5000, should not use it in this function.
Gotcha. Should the unused var be removed? Otherwise Reviewed-by: Dave
Jiang <dave.jiang(a)intel.com>
>
> Tony(zhu, xinzhan)
> Cube:SHZ1-3W-279
> iNet:8821-5077
>
> -----Original Message-----
> From: Jiang, Dave <dave.jiang(a)intel.com>
> Sent: Thursday, April 28, 2022 9:49 AM
> To: Zhu, Tony <tony.zhu(a)intel.com>; accel-config(a)lists.01.org
> Cc: Thomas, Ramesh <ramesh.thomas(a)intel.com>; Buchanan, Kenneth <kenneth.buchanan(a)intel.com>
> Subject: Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
>
>
> On 4/27/2022 6:31 PM, Tony Zhu wrote:
>> Dsa_test application will fail when when utilizing memory sizes larger
>> than 128MiB against multiple devices. The init of timeout should use
>> the input msec_timeout.
>>
>> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
> Ken, you want your sign off added?
>
>> ---
>> test/accel_test.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/accel_test.c b/test/accel_test.c index
>> 8487099..0468a00 100644
>> --- a/test/accel_test.c
>> +++ b/test/accel_test.c
>> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct completion_record *comp,
>> j++;
>> }
>> } else {
>> - unsigned long timeout = (ms_timeout * 1000000) * 3;
>> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
> Couple things. This looks like changed with the common code refactoring?
> Has that merged by Ramesh? If not then maybe do the diff before that code since we need to get this fix released. And then make sure the refactoring take this change into account.
>
> Also, did you compile test this? I don't see the var name ms_timeout changed to msec_timeout.
>
>> int r = 1;
>> unsigned long t = 0;
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 2:15 Zhu, Tony
0 siblings, 0 replies; 6+ messages in thread
From: Zhu, Tony @ 2022-04-28 2:15 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]
Dave,
The code base is pending branch. It included the common code refactoring.
I've tested it. msec_timeout is defined in the function.
int acctest_wait_on_desc_timeout(struct completion_record *comp,
struct acctest_context *ctx,
unsigned int msec_timeout)
unsigned int ms_timeout = 5000, should not use it in this function.
Tony(zhu, xinzhan)
Cube:SHZ1-3W-279
iNet:8821-5077
-----Original Message-----
From: Jiang, Dave <dave.jiang(a)intel.com>
Sent: Thursday, April 28, 2022 9:49 AM
To: Zhu, Tony <tony.zhu(a)intel.com>; accel-config(a)lists.01.org
Cc: Thomas, Ramesh <ramesh.thomas(a)intel.com>; Buchanan, Kenneth <kenneth.buchanan(a)intel.com>
Subject: Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
On 4/27/2022 6:31 PM, Tony Zhu wrote:
> Dsa_test application will fail when when utilizing memory sizes larger
> than 128MiB against multiple devices. The init of timeout should use
> the input msec_timeout.
>
> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
Ken, you want your sign off added?
> ---
> test/accel_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/accel_test.c b/test/accel_test.c index
> 8487099..0468a00 100644
> --- a/test/accel_test.c
> +++ b/test/accel_test.c
> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct completion_record *comp,
> j++;
> }
> } else {
> - unsigned long timeout = (ms_timeout * 1000000) * 3;
> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
Couple things. This looks like changed with the common code refactoring?
Has that merged by Ramesh? If not then maybe do the diff before that code since we need to get this fix released. And then make sure the refactoring take this change into account.
Also, did you compile test this? I don't see the var name ms_timeout changed to msec_timeout.
> int r = 1;
> unsigned long t = 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow
@ 2022-04-28 1:48 Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-04-28 1:48 UTC (permalink / raw)
To: accel-config
[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]
On 4/27/2022 6:31 PM, Tony Zhu wrote:
> Dsa_test application will fail when when utilizing memory sizes larger
> than 128MiB against multiple devices. The init of timeout should use
> the input msec_timeout.
>
> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
Ken, you want your sign off added?
> ---
> test/accel_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/accel_test.c b/test/accel_test.c
> index 8487099..0468a00 100644
> --- a/test/accel_test.c
> +++ b/test/accel_test.c
> @@ -331,7 +331,7 @@ int acctest_wait_on_desc_timeout(struct completion_record *comp,
> j++;
> }
> } else {
> - unsigned long timeout = (ms_timeout * 1000000) * 3;
> + unsigned long timeout = (msec_timeout * 1000000ul) * 3;
Couple things. This looks like changed with the common code refactoring?
Has that merged by Ramesh? If not then maybe do the diff before that
code since we need to get this fix released. And then make sure the
refactoring take this change into account.
Also, did you compile test this? I don't see the var name ms_timeout
changed to msec_timeout.
> int r = 1;
> unsigned long t = 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-28 15:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-28 2:31 [Accel-config] Re: [PATCH v1 1/1] accel-config/test: Fix description timeout calculation to overflow Zhu, Tony
-- strict thread matches above, loose matches on Subject: below --
2022-04-28 15:39 Dave Jiang
2022-04-28 15:32 Dave Jiang
2022-04-28 2:19 Dave Jiang
2022-04-28 2:15 Zhu, Tony
2022-04-28 1:48 Dave Jiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.