diff mbox

[PATCHv2,05/27] crypto: omap-sham: avoid executing tasklet where not needed

Message ID 1466601840-18486-6-git-send-email-t-kristo@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tero Kristo June 22, 2016, 1:23 p.m. UTC
Some of the call paths of OMAP SHA driver can avoid executing the next
step of the crypto queue under tasklet; instead, execute the next step
directly via function call. This avoids a costly round-trip via the
scheduler giving a slight performance boost.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 drivers/crypto/omap-sham.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Herbert Xu June 24, 2016, 10:30 a.m. UTC | #1
On Wed, Jun 22, 2016 at 04:23:38PM +0300, Tero Kristo wrote:
> Some of the call paths of OMAP SHA driver can avoid executing the next
> step of the crypto queue under tasklet; instead, execute the next step
> directly via function call. This avoids a costly round-trip via the
> scheduler giving a slight performance boost.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  drivers/crypto/omap-sham.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
> index 6247887..84a0027 100644
> --- a/drivers/crypto/omap-sham.c
> +++ b/drivers/crypto/omap-sham.c
> @@ -242,6 +242,8 @@ static struct omap_sham_drv sham = {
>  	.lock = __SPIN_LOCK_UNLOCKED(sham.lock),
>  };
>  
> +static void omap_sham_done_task(unsigned long data);
> +
>  static inline u32 omap_sham_read(struct omap_sham_dev *dd, u32 offset)
>  {
>  	return __raw_readl(dd->io_base + offset);
> @@ -1007,7 +1009,7 @@ static void omap_sham_finish_req(struct ahash_request *req, int err)
>  		req->base.complete(&req->base, err);
>  
>  	/* handle new request */
> -	tasklet_schedule(&dd->done_task);
> +	omap_sham_done_task((unsigned long)dd);
>  }

Hmm, what guarnatees that you won't run out of stack doing this?
Tero Kristo June 27, 2016, 5:04 a.m. UTC | #2
On 24/06/16 13:30, Herbert Xu wrote:
> On Wed, Jun 22, 2016 at 04:23:38PM +0300, Tero Kristo wrote:
>> Some of the call paths of OMAP SHA driver can avoid executing the next
>> step of the crypto queue under tasklet; instead, execute the next step
>> directly via function call. This avoids a costly round-trip via the
>> scheduler giving a slight performance boost.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   drivers/crypto/omap-sham.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
>> index 6247887..84a0027 100644
>> --- a/drivers/crypto/omap-sham.c
>> +++ b/drivers/crypto/omap-sham.c
>> @@ -242,6 +242,8 @@ static struct omap_sham_drv sham = {
>>   	.lock = __SPIN_LOCK_UNLOCKED(sham.lock),
>>   };
>>
>> +static void omap_sham_done_task(unsigned long data);
>> +
>>   static inline u32 omap_sham_read(struct omap_sham_dev *dd, u32 offset)
>>   {
>>   	return __raw_readl(dd->io_base + offset);
>> @@ -1007,7 +1009,7 @@ static void omap_sham_finish_req(struct ahash_request *req, int err)
>>   		req->base.complete(&req->base, err);
>>
>>   	/* handle new request */
>> -	tasklet_schedule(&dd->done_task);
>> +	omap_sham_done_task((unsigned long)dd);
>>   }
>
> Hmm, what guarnatees that you won't run out of stack doing this?

Good question, I had a deeper look at the driver and it seems you are 
right... It may result in a recursion loop within the driver. I will dig 
this further and break the recursion if it indeed can call itself 
indefinitely.

My knowledge of the crypto stack (+ these drivers unfortunately) is 
still pretty limited, I appreciate your reviews a lot.

-Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 6247887..84a0027 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -242,6 +242,8 @@  static struct omap_sham_drv sham = {
 	.lock = __SPIN_LOCK_UNLOCKED(sham.lock),
 };
 
+static void omap_sham_done_task(unsigned long data);
+
 static inline u32 omap_sham_read(struct omap_sham_dev *dd, u32 offset)
 {
 	return __raw_readl(dd->io_base + offset);
@@ -1007,7 +1009,7 @@  static void omap_sham_finish_req(struct ahash_request *req, int err)
 		req->base.complete(&req->base, err);
 
 	/* handle new request */
-	tasklet_schedule(&dd->done_task);
+	omap_sham_done_task((unsigned long)dd);
 }
 
 static int omap_sham_handle_queue(struct omap_sham_dev *dd,