2008-07-16 23:23:12

by Kim Phillips

[permalink] [raw]
Subject: [PATCH 4/6] crypto: talitos - fix GFP flag usage

use GFP_ATOMIC when necessary; use atomic_t when allocating submit_count.

Signed-off-by: Kim Phillips <[email protected]>
Acked-by: Lee Nipper <[email protected]>
---
drivers/crypto/talitos.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index fdb0680..79fdba2 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1015,6 +1015,8 @@ static struct ipsec_esp_edesc *ipsec_esp_edesc_alloc(struct aead_request *areq,
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
struct ipsec_esp_edesc *edesc;
int src_nents, dst_nents, alloc_len, dma_len;
+ gfp_t flags = areq->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ? GFP_KERNEL :
+ GFP_ATOMIC;

if (areq->cryptlen + ctx->authsize > TALITOS_MAX_DATA_LEN) {
dev_err(ctx->dev, "cryptlen exceeds h/w max limit\n");
@@ -1046,7 +1048,7 @@ static struct ipsec_esp_edesc *ipsec_esp_edesc_alloc(struct aead_request *areq,
alloc_len += icv_stashing ? ctx->authsize : 0;
}

- edesc = kmalloc(alloc_len, GFP_DMA);
+ edesc = kmalloc(alloc_len, GFP_DMA | flags);
if (!edesc) {
dev_err(ctx->dev, "could not allocate edescriptor\n");
return ERR_PTR(-ENOMEM);
@@ -1508,7 +1510,7 @@ static int talitos_probe(struct of_device *ofdev,
}
}

- priv->submit_count = kmalloc(sizeof(int) * priv->num_channels,
+ priv->submit_count = kmalloc(sizeof(atomic_t) * priv->num_channels,
GFP_KERNEL);
if (!priv->submit_count) {
dev_err(dev, "failed to allocate fifo submit count space\n");
--
1.5.6



2008-07-16 23:34:05

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage


On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:

> use GFP_ATOMIC when necessary; use atomic_t when allocating
> submit_count.

why?

- k

2008-07-17 12:18:05

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage

On Wed, Jul 16, 2008 at 06:33:45PM -0500, Kumar Gala wrote:
>
> On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:
>
> >use GFP_ATOMIC when necessary; use atomic_t when allocating
> >submit_count.
>
> why?

You mean why are atomics required? Yes that is a good question.

Kim?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-07-17 12:26:31

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage


On Jul 17, 2008, at 7:17 AM, Herbert Xu wrote:

> On Wed, Jul 16, 2008 at 06:33:45PM -0500, Kumar Gala wrote:
>>
>> On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:
>>
>>> use GFP_ATOMIC when necessary; use atomic_t when allocating
>>> submit_count.
>>
>> why?
>
> You mean why are atomics required? Yes that is a good question.

Yep. the commit message isn't explaining why, just what :)

> Kim?

- k

2008-07-17 15:28:32

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage

On Thu, 17 Jul 2008 07:26:14 -0500
Kumar Gala <[email protected]> wrote:

>
> On Jul 17, 2008, at 7:17 AM, Herbert Xu wrote:
>
> > On Wed, Jul 16, 2008 at 06:33:45PM -0500, Kumar Gala wrote:
> >>
> >> On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:
> >>
> >>> use GFP_ATOMIC when necessary; use atomic_t when allocating
> >>> submit_count.
> >>
> >> why?
> >
> > You mean why are atomics required? Yes that is a good question.
>
> Yep. the commit message isn't explaining why, just what :)

In honouring requests that don't have the CRYPTO_TFM_REQ_MAY_SLEEP set,
afaict, it's the standard non-wait variant GFP that drivers use (see
the ixp4xx driver for e.g.).

Kim

2008-07-17 15:52:03

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage


On Jul 17, 2008, at 10:27 AM, Kim Phillips wrote:

> On Thu, 17 Jul 2008 07:26:14 -0500
> Kumar Gala <[email protected]> wrote:
>
>>
>> On Jul 17, 2008, at 7:17 AM, Herbert Xu wrote:
>>
>>> On Wed, Jul 16, 2008 at 06:33:45PM -0500, Kumar Gala wrote:
>>>>
>>>> On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:
>>>>
>>>>> use GFP_ATOMIC when necessary; use atomic_t when allocating
>>>>> submit_count.
>>>>
>>>> why?
>>>
>>> You mean why are atomics required? Yes that is a good question.
>>
>> Yep. the commit message isn't explaining why, just what :)
>
> In honouring requests that don't have the CRYPTO_TFM_REQ_MAY_SLEEP
> set,
> afaict, it's the standard non-wait variant GFP that drivers use (see
> the ixp4xx driver for e.g.).

so GFP_ATOMIC and atomic_t aren't related. I can understand the need
for GFP_ATOMIC, but I don't get why something needs to be declared
atomic_t.

- k

2008-07-17 16:24:44

by Kim Phillips

[permalink] [raw]
Subject: Re: [PATCH 4/6] crypto: talitos - fix GFP flag usage

On Thu, 17 Jul 2008 10:51:43 -0500
Kumar Gala <[email protected]> wrote:

>
> On Jul 17, 2008, at 10:27 AM, Kim Phillips wrote:
>
> > On Thu, 17 Jul 2008 07:26:14 -0500
> > Kumar Gala <[email protected]> wrote:
> >
> >>
> >> On Jul 17, 2008, at 7:17 AM, Herbert Xu wrote:
> >>
> >>> On Wed, Jul 16, 2008 at 06:33:45PM -0500, Kumar Gala wrote:
> >>>>
> >>>> On Jul 16, 2008, at 6:22 PM, Kim Phillips wrote:
> >>>>
> >>>>> use GFP_ATOMIC when necessary; use atomic_t when allocating
> >>>>> submit_count.
> >>>>
> >>>> why?
> >>>
> >>> You mean why are atomics required? Yes that is a good question.
> >>
> >> Yep. the commit message isn't explaining why, just what :)
> >
> > In honouring requests that don't have the CRYPTO_TFM_REQ_MAY_SLEEP
> > set,
> > afaict, it's the standard non-wait variant GFP that drivers use (see
> > the ixp4xx driver for e.g.).
>
> so GFP_ATOMIC and atomic_t aren't related. I can understand the need
> for GFP_ATOMIC, but I don't get why something needs to be declared
> atomic_t.

I see what the source of confusion is now; it appears I missed this
atomic_t clean-up when making the patchseries; the atomic_t fix belongs
in the previous "preempt overflow interrupts" patch (3/6).

atomic_t is needed for atomic operations which protect resource
contention on submit_count, which subsequently represents submission
slot availability on a particular SEC channel for multiple, potentially
simultaneous, users.

Kim