From: Tero Kristo <t-kristo@ti.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: <linux-omap@vger.kernel.org>, <linux-crypto@vger.kernel.org>,
<tony@atomide.com>, <davem@davemloft.net>, <lokeshvutla@ti.com>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCHv2 05/27] crypto: omap-sham: avoid executing tasklet where not needed
Date: Mon, 27 Jun 2016 08:04:24 +0300 [thread overview]
Message-ID: <5770B3D8.9010909@ti.com> (raw)
In-Reply-To: <20160624103042.GA19571@gondor.apana.org.au>
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
WARNING: multiple messages have this Message-ID (diff)
From: Tero Kristo <t-kristo@ti.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-omap@vger.kernel.org, linux-crypto@vger.kernel.org,
tony@atomide.com, davem@davemloft.net, lokeshvutla@ti.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2 05/27] crypto: omap-sham: avoid executing tasklet where not needed
Date: Mon, 27 Jun 2016 08:04:24 +0300 [thread overview]
Message-ID: <5770B3D8.9010909@ti.com> (raw)
In-Reply-To: <20160624103042.GA19571@gondor.apana.org.au>
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
WARNING: multiple messages have this Message-ID (diff)
From: t-kristo@ti.com (Tero Kristo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 05/27] crypto: omap-sham: avoid executing tasklet where not needed
Date: Mon, 27 Jun 2016 08:04:24 +0300 [thread overview]
Message-ID: <5770B3D8.9010909@ti.com> (raw)
In-Reply-To: <20160624103042.GA19571@gondor.apana.org.au>
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
next prev parent reply other threads:[~2016-06-27 5:05 UTC|newest]
Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-22 13:23 [PATCHv2 00/27] crypto: fixes for omap family of devices Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 01/27] crypto: omap-sham: use runtime_pm autosuspend for clock handling Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-24 13:30 ` Herbert Xu
2016-06-24 13:30 ` Herbert Xu
2016-06-22 13:23 ` [PATCHv2 02/27] crypto: omap-sham: change queue size from 1 to 10 Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 03/27] crypto: omap: do not call dmaengine_terminate_all Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 04/27] crypto: omap-sham: set sw fallback to 240 bytes Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 05/27] crypto: omap-sham: avoid executing tasklet where not needed Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-24 10:30 ` Herbert Xu
2016-06-24 10:30 ` Herbert Xu
2016-06-27 5:04 ` Tero Kristo [this message]
2016-06-27 5:04 ` Tero Kristo
2016-06-27 5:04 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 06/27] crypto: ahash: increase the maximum allowed statesize Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-24 10:32 ` Herbert Xu
2016-06-24 10:32 ` Herbert Xu
2016-06-27 4:58 ` Tero Kristo
2016-06-27 4:58 ` Tero Kristo
2016-06-27 4:58 ` Tero Kristo
2016-06-27 5:00 ` Herbert Xu
2016-06-27 5:00 ` Herbert Xu
2016-07-04 9:17 ` Tero Kristo
2016-07-04 9:17 ` Tero Kristo
2016-07-04 9:19 ` Herbert Xu
2016-07-04 9:19 ` Herbert Xu
2016-07-04 9:27 ` Tero Kristo
2016-07-04 9:27 ` Tero Kristo
2016-07-04 9:27 ` Tero Kristo
2016-07-04 9:42 ` Herbert Xu
2016-07-04 9:42 ` Herbert Xu
2016-06-22 13:23 ` [PATCHv2 07/27] crypto: omap-sham: implement context export/import APIs Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 08/27] crypto: omap-des: Fix support for unequal lengths Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 09/27] crypto: omap-aes - Fix enabling clocks Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 10/27] crypto: omap-aes: Add support for multiple cores Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 11/27] crypto: omap-aes: Add fallback support Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 12/27] crypto: engine: avoid unnecessary context switches Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 13/27] crypto: omap-aes: fix crypto engine initialization order Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 14/27] crypto: omap-des: " Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 15/27] ARM: dts: DRA7: Add DT node for DES IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 16/27] ARM: dts: DRA7: Add DT nodes for AES IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 17/27] ARM: dts: DRA7: Add support for SHA IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 18/27] ARM: dts: DRA7: Add DT node for RNG IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 19/27] ARM: dts: AM43xx: clk: Add RNG clk node Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 20/27] ARM: dts: AM43xx: Add node for RNG Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 21/27] ARM: DRA7: hwmod: Add data for DES IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 22/27] ARM: DRA7: hwmod: Add data for AES IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 23/27] ARM: DRA7: hwmod: Add data for SHA IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 24/27] ARM: DRA7: hwmod: Add data for RNG IP Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 25/27] ARM: OMAP: DRA7xx: Make L4SEC clock domain SWSUP only Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` [PATCHv2 26/27] ARM: AM43xx: hwmod: Add data for DES Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:23 ` Tero Kristo
2016-06-22 13:24 ` [PATCHv2 27/27] ARM: AMx3xx: hwmod: Add data for RNG Tero Kristo
2016-06-22 13:24 ` Tero Kristo
2016-06-22 13:24 ` Tero Kristo
2016-06-23 4:49 ` [PATCHv2 00/27] crypto: fixes for omap family of devices Tony Lindgren
2016-06-23 4:49 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5770B3D8.9010909@ti.com \
--to=t-kristo@ti.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=lokeshvutla@ti.com \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.