diff mbox

[v2,2/8] crypto: scompress - use sgl_alloc() and sgl_free()

Message ID 20171016224940.1332-3-bart.vanassche@wdc.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Bart Van Assche Oct. 16, 2017, 10:49 p.m. UTC
Use the sgl_alloc() and sgl_free() functions instead of open coding
these functions.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
 crypto/Kconfig     |  1 +
 crypto/scompress.c | 51 ++-------------------------------------------------
 2 files changed, 3 insertions(+), 49 deletions(-)

Comments

Hannes Reinecke Oct. 17, 2017, 6:07 a.m. UTC | #1
On 10/17/2017 12:49 AM, Bart Van Assche wrote:
> Use the sgl_alloc() and sgl_free() functions instead of open coding
> these functions.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>  crypto/Kconfig     |  1 +
>  crypto/scompress.c | 51 ++-------------------------------------------------
>  2 files changed, 3 insertions(+), 49 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
Bart Van Assche Nov. 1, 2017, 2:50 p.m. UTC | #2
On Mon, 2017-10-16 at 15:49 -0700, Bart Van Assche wrote:
> Use the sgl_alloc() and sgl_free() functions instead of open coding

> these functions.

> 

> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Herbert Xu <herbert@gondor.apana.org.au>


Ard and/or Herbert, can you please have a look at this patch and let us know
whether or not it looks fine to you?

Thanks,

Bart.
Ard Biesheuvel Nov. 1, 2017, 3:17 p.m. UTC | #3
On 1 November 2017 at 14:50, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Mon, 2017-10-16 at 15:49 -0700, Bart Van Assche wrote:
>> Use the sgl_alloc() and sgl_free() functions instead of open coding
>> these functions.
>>
>> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>
> Ard and/or Herbert, can you please have a look at this patch and let us know
> whether or not it looks fine to you?
>

The patch itself does not look unreasonable, but I can't find
sgl_alloc() anywhere in the source tree. Given that you have cc'ed me
on this patch only, I can only assume that you are adding this as part
of the series, but without any context, I can't really review this,
sorry.
Bart Van Assche Nov. 1, 2017, 3:45 p.m. UTC | #4
On Wed, 2017-11-01 at 15:17 +0000, Ard Biesheuvel wrote:
> On 1 November 2017 at 14:50, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:

> > On Mon, 2017-10-16 at 15:49 -0700, Bart Van Assche wrote:

> > > Use the sgl_alloc() and sgl_free() functions instead of open coding

> > > these functions.

> > > 

> > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > > Cc: Herbert Xu <herbert@gondor.apana.org.au>

> > 

> > Ard and/or Herbert, can you please have a look at this patch and let us know

> > whether or not it looks fine to you?

> 

> The patch itself does not look unreasonable, but I can't find

> sgl_alloc() anywhere in the source tree. Given that you have cc'ed me

> on this patch only, I can only assume that you are adding this as part

> of the series, but without any context, I can't really review this,

> sorry.


Hello Ard,

Do you expect to be Cc-ed personally or is Cc-ing the linux-crypto mailing
list sufficient? The linux-crypto mailing list was Cc-ed for the entire patch
series as one can see here:
https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg28485.html.

Thanks,

Bart.
Ard Biesheuvel Nov. 1, 2017, 3:51 p.m. UTC | #5
On 1 November 2017 at 15:45, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Wed, 2017-11-01 at 15:17 +0000, Ard Biesheuvel wrote:
>> On 1 November 2017 at 14:50, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
>> > On Mon, 2017-10-16 at 15:49 -0700, Bart Van Assche wrote:
>> > > Use the sgl_alloc() and sgl_free() functions instead of open coding
>> > > these functions.
>> > >
>> > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
>> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > > Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> >
>> > Ard and/or Herbert, can you please have a look at this patch and let us know
>> > whether or not it looks fine to you?
>>
>> The patch itself does not look unreasonable, but I can't find
>> sgl_alloc() anywhere in the source tree. Given that you have cc'ed me
>> on this patch only, I can only assume that you are adding this as part
>> of the series, but without any context, I can't really review this,
>> sorry.
>
> Hello Ard,
>
> Do you expect to be Cc-ed personally or is Cc-ing the linux-crypto mailing
> list sufficient? The linux-crypto mailing list was Cc-ed for the entire patch
> series as one can see here:
> https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg28485.html.
>

I guess people's opinions may differ regarding what they want to be
cc'ed on, but in general, you should at least cc everyone on the cover
letter if you cc them on individual patches, and in my case, I'd
rather have the whole series even if only a single patch is relevant
to me.
diff mbox

Patch

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 0a121f9ddf8e..a0667dd284ff 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -105,6 +105,7 @@  config CRYPTO_KPP
 config CRYPTO_ACOMP2
 	tristate
 	select CRYPTO_ALGAPI2
+	select SGL_ALLOC
 
 config CRYPTO_ACOMP
 	tristate
diff --git a/crypto/scompress.c b/crypto/scompress.c
index 2075e2c4e7df..968bbcf65c94 100644
--- a/crypto/scompress.c
+++ b/crypto/scompress.c
@@ -140,53 +140,6 @@  static int crypto_scomp_init_tfm(struct crypto_tfm *tfm)
 	return ret;
 }
 
-static void crypto_scomp_sg_free(struct scatterlist *sgl)
-{
-	int i, n;
-	struct page *page;
-
-	if (!sgl)
-		return;
-
-	n = sg_nents(sgl);
-	for_each_sg(sgl, sgl, n, i) {
-		page = sg_page(sgl);
-		if (page)
-			__free_page(page);
-	}
-
-	kfree(sgl);
-}
-
-static struct scatterlist *crypto_scomp_sg_alloc(size_t size, gfp_t gfp)
-{
-	struct scatterlist *sgl;
-	struct page *page;
-	int i, n;
-
-	n = ((size - 1) >> PAGE_SHIFT) + 1;
-
-	sgl = kmalloc_array(n, sizeof(struct scatterlist), gfp);
-	if (!sgl)
-		return NULL;
-
-	sg_init_table(sgl, n);
-
-	for (i = 0; i < n; i++) {
-		page = alloc_page(gfp);
-		if (!page)
-			goto err;
-		sg_set_page(sgl + i, page, PAGE_SIZE, 0);
-	}
-
-	return sgl;
-
-err:
-	sg_mark_end(sgl + i);
-	crypto_scomp_sg_free(sgl);
-	return NULL;
-}
-
 static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
 {
 	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
@@ -220,7 +173,7 @@  static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
 					      scratch_dst, &req->dlen, *ctx);
 	if (!ret) {
 		if (!req->dst) {
-			req->dst = crypto_scomp_sg_alloc(req->dlen, GFP_ATOMIC);
+			req->dst = sgl_alloc(req->dlen, GFP_ATOMIC, NULL);
 			if (!req->dst)
 				goto out;
 		}
@@ -274,7 +227,7 @@  int crypto_init_scomp_ops_async(struct crypto_tfm *tfm)
 
 	crt->compress = scomp_acomp_compress;
 	crt->decompress = scomp_acomp_decompress;
-	crt->dst_free = crypto_scomp_sg_free;
+	crt->dst_free = sgl_free;
 	crt->reqsize = sizeof(void *);
 
 	return 0;