| Message ID | 20170521060952.19055-1-prasannatsmkumar@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Herbert Xu |
| Headers | show |
On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> wrote: > As cra_ctxsize is set but the allocated space is not used, set it 0. Why do you think it is not used? Did you test our change on hardware? Best regards, Krzysztof > > Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> > --- > drivers/crypto/exynos-rng.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c > index 451620b..d009df6 100644 > --- a/drivers/crypto/exynos-rng.c > +++ b/drivers/crypto/exynos-rng.c > @@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm) > .cra_name = "stdrng", > .cra_driver_name = "exynos_rng", > .cra_priority = 100, > - .cra_ctxsize = sizeof(struct exynos_rng_ctx), > + .cra_ctxsize = 0, > .cra_module = THIS_MODULE, > .cra_init = exynos_rng_kcapi_init, > } > -- > 1.8.5.6 >
Hi Krzysztof On 21 May 2017 at 11:56, Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan > <prasannatsmkumar@gmail.com> wrote: >> As cra_ctxsize is set but the allocated space is not used, set it 0. > > Why do you think it is not used? Did you test our change on hardware? Had a look at the crypto rng code. I think the additional size is used to store driver private data. But this driver does not store any private data in the crypto_tfm structure so I think the 'cra_ctxsize' can be safely set to 0. I do not have access to the hardware, did not test the change. Sorry I forgot to mention that. > Best regards, > Krzysztof > >> >> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> >> --- >> drivers/crypto/exynos-rng.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c >> index 451620b..d009df6 100644 >> --- a/drivers/crypto/exynos-rng.c >> +++ b/drivers/crypto/exynos-rng.c >> @@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm) >> .cra_name = "stdrng", >> .cra_driver_name = "exynos_rng", >> .cra_priority = 100, >> - .cra_ctxsize = sizeof(struct exynos_rng_ctx), >> + .cra_ctxsize = 0, >> .cra_module = THIS_MODULE, >> .cra_init = exynos_rng_kcapi_init, >> } >> -- >> 1.8.5.6 >> Regards, PrasannaKumar
On Sun, May 21, 2017 at 9:11 AM, PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> wrote: > Hi Krzysztof > > On 21 May 2017 at 11:56, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan >> <prasannatsmkumar@gmail.com> wrote: >>> As cra_ctxsize is set but the allocated space is not used, set it 0. >> >> Why do you think it is not used? Did you test our change on hardware? > > Had a look at the crypto rng code. I think the additional size is used > to store driver private data. But this driver does not store any > private data in the crypto_tfm structure so I think the 'cra_ctxsize' > can be safely set to 0. Then from where does crypto_tfm_ctx() get its memory? > I do not have access to the hardware, did not test the change. Sorry I > forgot to mention that. That is quite important... By default everything must be tested so if you are skipping this step then please mark the patch respectively so others will provide testing. Best regards, Krzysztof
On 21 May 2017 at 12:44, Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Sun, May 21, 2017 at 9:11 AM, PrasannaKumar Muralidharan > <prasannatsmkumar@gmail.com> wrote: >> Hi Krzysztof >> >> On 21 May 2017 at 11:56, Krzysztof Kozlowski <krzk@kernel.org> wrote: >>> On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan >>> <prasannatsmkumar@gmail.com> wrote: >>>> As cra_ctxsize is set but the allocated space is not used, set it 0. >>> >>> Why do you think it is not used? Did you test our change on hardware? >> >> Had a look at the crypto rng code. I think the additional size is used >> to store driver private data. But this driver does not store any >> private data in the crypto_tfm structure so I think the 'cra_ctxsize' >> can be safely set to 0. > > Then from where does crypto_tfm_ctx() get its memory? Ah, yes. Thanks for pointing out. I overlooked this. My patch is completely wrong. >> I do not have access to the hardware, did not test the change. Sorry I >> forgot to mention that. > > That is quite important... By default everything must be tested so if > you are skipping this step then please mark the patch respectively so > others will provide testing. Sure. Will keep that in mind. Instead of marking it as a [PATCH] should I use something else for this? Will it make things easier? Thanks, PrasannaKumar
On Mon, May 22, 2017 at 5:44 AM, PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> wrote: >>> I do not have access to the hardware, did not test the change. Sorry I >>> forgot to mention that. >> >> That is quite important... By default everything must be tested so if >> you are skipping this step then please mark the patch respectively so >> others will provide testing. > > Sure. Will keep that in mind. Instead of marking it as a [PATCH] > should I use something else for this? Will it make things easier? If sending untested patch is really necessary, then mark it either as RFT instead of PATCH or add a comment about it after --- separator. Best regards, Krzysztof
diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c index 451620b..d009df6 100644 --- a/drivers/crypto/exynos-rng.c +++ b/drivers/crypto/exynos-rng.c @@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm) .cra_name = "stdrng", .cra_driver_name = "exynos_rng", .cra_priority = 100, - .cra_ctxsize = sizeof(struct exynos_rng_ctx), + .cra_ctxsize = 0, .cra_module = THIS_MODULE, .cra_init = exynos_rng_kcapi_init, }
As cra_ctxsize is set but the allocated space is not used, set it 0. Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> --- drivers/crypto/exynos-rng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)