[PATCH] tee: optee: remove need_resched() before cond_resched()

[PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

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

Testing need_resched() before cond_resched() is not needed as an
equivalent test is done internally in cond_resched(). So drop the
need_resched() test.

Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 drivers/tee/optee/call.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 780d7c4fd756..7a77e375b503 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -149,8 +149,7 @@ u32 optee_do_call_with_arg(struct tee_context *ctx, phys_addr_t parg)
 			 */
 			optee_cq_wait_for_completion(&optee->call_queue, &w);
 		} else if (OPTEE_SMC_RETURN_IS_RPC(res.a0)) {
-			if (need_resched())
-				cond_resched();
+			cond_resched();
 			param.a0 = res.a0;
 			param.a1 = res.a1;
 			param.a2 = res.a2;
-- 
2.25.1

[PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

Testing need_resched() before cond_resched() is not needed as an
equivalent test is done internally in cond_resched(). So drop the
need_resched() test.

Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 drivers/tee/optee/call.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 780d7c4fd756..7a77e375b503 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -149,8 +149,7 @@ u32 optee_do_call_with_arg(struct tee_context *ctx, phys_addr_t parg)
 			 */
 			optee_cq_wait_for_completion(&optee->call_queue, &w);
 		} else if (OPTEE_SMC_RETURN_IS_RPC(res.a0)) {
-			if (need_resched())
-				cond_resched();
+			cond_resched();
 			param.a0 = res.a0;
 			param.a1 = res.a1;
 			param.a2 = res.a2;
-- 
2.25.1

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

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

Hi Rouven and Sumit,

On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Testing need_resched() before cond_resched() is not needed as an
> equivalent test is done internally in cond_resched(). So drop the
> need_resched() test.
>
> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/tee/optee/call.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Can you please test to see that this works in your setups too?

Thanks,
Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

Hi Rouven and Sumit,

On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Testing need_resched() before cond_resched() is not needed as an
> equivalent test is done internally in cond_resched(). So drop the
> need_resched() test.
>
> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  drivers/tee/optee/call.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Can you please test to see that this works in your setups too?

Thanks,
Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

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

Hi Rouven and Sumit,

On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
<op-tee@lists.trustedfirmware.org> wrote:
>
> Hi Rouven and Sumit,
>
> On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
> <jens.wiklander@linaro.org> wrote:
> >
> > Testing need_resched() before cond_resched() is not needed as an
> > equivalent test is done internally in cond_resched(). So drop the
> > need_resched() test.
> >
> > Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > ---
> >  drivers/tee/optee/call.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Can you please test to see that this works in your setups too?

Does this work for you? I'd like to get this patch ready for v5.12.

Thanks,
Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Rouven Czerwinski]

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

On 29.01.21 14:29, Jens Wiklander wrote:
> Hi Rouven and Sumit,
> 
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
>>
>> Hi Rouven and Sumit,
>>
>> On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
>> <jens.wiklander@linaro.org> wrote:
>>>
>>> Testing need_resched() before cond_resched() is not needed as an
>>> equivalent test is done internally in cond_resched(). So drop the
>>> need_resched() test.
>>>
>>> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>> ---
>>>   drivers/tee/optee/call.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Can you please test to see that this works in your setups too?
> 
> Does this work for you? I'd like to get this patch ready for v5.12.

Sorry I did not get to this yet, I'll need to modify the existing 
kernelconfig again to reproduce the buggy configuration. I'll get this 
done by monday.

Regards,
Rouven

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Arnd Bergmann]

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

On Mon, Jan 25, 2021 at 10:54 AM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Testing need_resched() before cond_resched() is not needed as an
> equivalent test is done internally in cond_resched(). So drop the
> need_resched() test.
>
> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Rouven Czerwinski]

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

Hi Jens,

On 29.01.21 14:29, Jens Wiklander wrote:
> Hi Rouven and Sumit,
> 
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
>>
>> Hi Rouven and Sumit,
>>
>> On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
>> <jens.wiklander@linaro.org> wrote:
>>>
>>> Testing need_resched() before cond_resched() is not needed as an
>>> equivalent test is done internally in cond_resched(). So drop the
>>> need_resched() test.
>>>
>>> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>> ---
>>>   drivers/tee/optee/call.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Can you please test to see that this works in your setups too?
> 
> Does this work for you? I'd like to get this patch ready for v5.12.

Yes it does. I also looked at it right after Arnds mail, so for future 
reference:


cond_resched()

   _cond_resched() (ifndef CONFIG_PREEMPTION)

     should_resched(0)

       tif_need_resched() (+ preempt_count == 0 for current thread)



while:



need_resched()

   tif_need_resched()


You can add my
Reviewed-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

I also retested on the STM32-DK2 and the problem remains fixed:

Tested-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Regrads and thanks for the patch,
Rouven

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Rouven Czerwinski]

Hi Jens,

On 29.01.21 14:29, Jens Wiklander wrote:
> Hi Rouven and Sumit,
> 
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
>>
>> Hi Rouven and Sumit,
>>
>> On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
>> <jens.wiklander@linaro.org> wrote:
>>>
>>> Testing need_resched() before cond_resched() is not needed as an
>>> equivalent test is done internally in cond_resched(). So drop the
>>> need_resched() test.
>>>
>>> Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
>>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>>> ---
>>>   drivers/tee/optee/call.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Can you please test to see that this works in your setups too?
> 
> Does this work for you? I'd like to get this patch ready for v5.12.

Yes it does. I also looked at it right after Arnds mail, so for future 
reference:


cond_resched()

   _cond_resched() (ifndef CONFIG_PREEMPTION)

     should_resched(0)

       tif_need_resched() (+ preempt_count == 0 for current thread)



while:



need_resched()

   tif_need_resched()


You can add my
Reviewed-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

I also retested on the STM32-DK2 and the problem remains fixed:

Tested-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Regrads and thanks for the patch,
Rouven

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Sumit Garg]

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

Hi Jens,

On Fri, 29 Jan 2021 at 18:59, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi Rouven and Sumit,
>
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
> >
> > Hi Rouven and Sumit,
> >
> > On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > Testing need_resched() before cond_resched() is not needed as an
> > > equivalent test is done internally in cond_resched(). So drop the
> > > need_resched() test.
> > >
> > > Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/call.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > Can you please test to see that this works in your setups too?
>
> Does this work for you? I'd like to get this patch ready for v5.12.

It does work for me as well.

Tested-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

>
> Thanks,
> Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Sumit Garg]

Hi Jens,

On Fri, 29 Jan 2021 at 18:59, Jens Wiklander <jens.wiklander@linaro.org> wrote:
>
> Hi Rouven and Sumit,
>
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
> >
> > Hi Rouven and Sumit,
> >
> > On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > Testing need_resched() before cond_resched() is not needed as an
> > > equivalent test is done internally in cond_resched(). So drop the
> > > need_resched() test.
> > >
> > > Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/call.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > Can you please test to see that this works in your setups too?
>
> Does this work for you? I'd like to get this patch ready for v5.12.

It does work for me as well.

Tested-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

>
> Thanks,
> Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

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

On Fri, Jan 29, 2021 at 02:29:33PM +0100, Jens Wiklander wrote:
> Hi Rouven and Sumit,
> 
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
> >
> > Hi Rouven and Sumit,
> >
> > On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > Testing need_resched() before cond_resched() is not needed as an
> > > equivalent test is done internally in cond_resched(). So drop the
> > > need_resched() test.
> > >
> > > Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/call.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > Can you please test to see that this works in your setups too?
> 
> Does this work for you? I'd like to get this patch ready for v5.12.

Thanks for the help with this patch. I'll submit this up for v5.12.

Cheers,
Jens

Re: [PATCH] tee: optee: remove need_resched() before cond_resched()

[Jens Wiklander]

On Fri, Jan 29, 2021 at 02:29:33PM +0100, Jens Wiklander wrote:
> Hi Rouven and Sumit,
> 
> On Mon, Jan 25, 2021 at 10:58 AM Jens Wiklander via OP-TEE
> <op-tee@lists.trustedfirmware.org> wrote:
> >
> > Hi Rouven and Sumit,
> >
> > On Mon, Jan 25, 2021 at 10:55 AM Jens Wiklander
> > <jens.wiklander@linaro.org> wrote:
> > >
> > > Testing need_resched() before cond_resched() is not needed as an
> > > equivalent test is done internally in cond_resched(). So drop the
> > > need_resched() test.
> > >
> > > Fixes: dcb3b06d9c34 ("tee: optee: replace might_sleep with cond_resched")
> > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> > > ---
> > >  drivers/tee/optee/call.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > Can you please test to see that this works in your setups too?
> 
> Does this work for you? I'd like to get this patch ready for v5.12.

Thanks for the help with this patch. I'll submit this up for v5.12.

Cheers,
Jens