[CRYPTO] tcrypt: Move sg_init_table out of timing loops
This patch moves the sg_init_table out of the timing loops for hash
algorithms so that it doesn't impact on the speed test results.
Signed-off-by: Herbert Xu <[email protected]>
---
crypto/tcrypt.c | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index c457bdb..24141fb 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -572,9 +572,11 @@ static int test_hash_jiffies_digest(struct hash_desc *desc, char *p, int blen,
int bcount;
int ret;
+ sg_init_table(sg, 1);
+
for (start = jiffies, end = start + sec * HZ, bcount = 0;
time_before(jiffies, end); bcount++) {
- sg_init_one(sg, p, blen);
+ sg_set_buf(sg, p, blen);
ret = crypto_hash_digest(desc, sg, blen, out);
if (ret)
return ret;
@@ -597,13 +599,15 @@ static int test_hash_jiffies(struct hash_desc *desc, char *p, int blen,
if (plen == blen)
return test_hash_jiffies_digest(desc, p, blen, out, sec);
+ sg_init_table(sg, 1);
+
for (start = jiffies, end = start + sec * HZ, bcount = 0;
time_before(jiffies, end); bcount++) {
ret = crypto_hash_init(desc);
if (ret)
return ret;
for (pcount = 0; pcount < blen; pcount += plen) {
- sg_init_one(sg, p + pcount, plen);
+ sg_set_buf(sg, p + pcount, plen);
ret = crypto_hash_update(desc, sg, plen);
if (ret)
return ret;
@@ -628,12 +632,14 @@ static int test_hash_cycles_digest(struct hash_desc *desc, char *p, int blen,
int i;
int ret;
+ sg_init_table(sg, 1);
+
local_bh_disable();
local_irq_disable();
/* Warm-up run. */
for (i = 0; i < 4; i++) {
- sg_init_one(sg, p, blen);
+ sg_set_buf(sg, p, blen);
ret = crypto_hash_digest(desc, sg, blen, out);
if (ret)
goto out;
@@ -645,7 +651,7 @@ static int test_hash_cycles_digest(struct hash_desc *desc, char *p, int blen,
start = get_cycles();
- sg_init_one(sg, p, blen);
+ sg_set_buf(sg, p, blen);
ret = crypto_hash_digest(desc, sg, blen, out);
if (ret)
goto out;
@@ -679,6 +685,8 @@ static int test_hash_cycles(struct hash_desc *desc, char *p, int blen,
if (plen == blen)
return test_hash_cycles_digest(desc, p, blen, out);
+ sg_init_table(sg, 1);
+
local_bh_disable();
local_irq_disable();
@@ -688,7 +696,7 @@ static int test_hash_cycles(struct hash_desc *desc, char *p, int blen,
if (ret)
goto out;
for (pcount = 0; pcount < blen; pcount += plen) {
- sg_init_one(sg, p + pcount, plen);
+ sg_set_buf(sg, p + pcount, plen);
ret = crypto_hash_update(desc, sg, plen);
if (ret)
goto out;
@@ -708,7 +716,7 @@ static int test_hash_cycles(struct hash_desc *desc, char *p, int blen,
if (ret)
goto out;
for (pcount = 0; pcount < blen; pcount += plen) {
- sg_init_one(sg, p + pcount, plen);
+ sg_set_buf(sg, p + pcount, plen);
ret = crypto_hash_update(desc, sg, plen);
if (ret)
goto out;
From: Herbert Xu <[email protected]>
Date: Fri, 26 Oct 2007 23:00:04 +0800
> [CRYPTO] tcrypt: Move sg_init_table out of timing loops
>
> This patch moves the sg_init_table out of the timing loops for hash
> algorithms so that it doesn't impact on the speed test results.
>
> Signed-off-by: Herbert Xu <[email protected]>
Applied, thanks!
On Fri, Oct 26 2007, Herbert Xu wrote:
> [CRYPTO] tcrypt: Move sg_init_table out of timing loops
>
> This patch moves the sg_init_table out of the timing loops for hash
> algorithms so that it doesn't impact on the speed test results.
Wouldn't it be better to just make sg_init_one() call sg_init_table?
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 4571231..ccc55a6 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -202,28 +202,6 @@ static inline void __sg_mark_end(struct scatterlist *sg)
}
/**
- * sg_init_one - Initialize a single entry sg list
- * @sg: SG entry
- * @buf: Virtual address for IO
- * @buflen: IO length
- *
- * Notes:
- * This should not be used on a single entry that is part of a larger
- * table. Use sg_init_table() for that.
- *
- **/
-static inline void sg_init_one(struct scatterlist *sg, const void *buf,
- unsigned int buflen)
-{
- memset(sg, 0, sizeof(*sg));
-#ifdef CONFIG_DEBUG_SG
- sg->sg_magic = SG_MAGIC;
-#endif
- sg_mark_end(sg, 1);
- sg_set_buf(sg, buf, buflen);
-}
-
-/**
* sg_init_table - Initialize SG table
* @sgl: The SG table
* @nents: Number of entries in table
@@ -247,6 +225,24 @@ static inline void sg_init_table(struct scatterlist *sgl, unsigned int nents)
}
/**
+ * sg_init_one - Initialize a single entry sg list
+ * @sg: SG entry
+ * @buf: Virtual address for IO
+ * @buflen: IO length
+ *
+ * Notes:
+ * This should not be used on a single entry that is part of a larger
+ * table. Use sg_init_table() for that.
+ *
+ **/
+static inline void sg_init_one(struct scatterlist *sg, const void *buf,
+ unsigned int buflen)
+{
+ sg_init_table(sg, 1);
+ sg_set_buf(sg, buf, buflen);
+}
+
+/**
* sg_phys - Return physical address of an sg entry
* @sg: SG entry
*
--
Jens Axboe
On Mon, Oct 29, 2007 at 09:16:27PM +0100, Jens Axboe wrote:
> On Fri, Oct 26 2007, Herbert Xu wrote:
> > [CRYPTO] tcrypt: Move sg_init_table out of timing loops
> >
> > This patch moves the sg_init_table out of the timing loops for hash
> > algorithms so that it doesn't impact on the speed test results.
>
> Wouldn't it be better to just make sg_init_one() call sg_init_table?
This looks fine to me although I think it's orthogonal to the
patch you were quoting :)
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
On Tue, Oct 30 2007, Herbert Xu wrote:
> On Mon, Oct 29, 2007 at 09:16:27PM +0100, Jens Axboe wrote:
> > On Fri, Oct 26 2007, Herbert Xu wrote:
> > > [CRYPTO] tcrypt: Move sg_init_table out of timing loops
> > >
> > > This patch moves the sg_init_table out of the timing loops for hash
> > > algorithms so that it doesn't impact on the speed test results.
> >
> > Wouldn't it be better to just make sg_init_one() call sg_init_table?
>
> This looks fine to me although I think it's orthogonal to the
> patch you were quoting :)
How so? The reason you changed it to sg_init_table() + sg_set_buf() is
exactly because sg_init_one() didn't properly init the entry (as they
name promised).
--
Jens Axboe
On Mon, Oct 29 2007 at 22:16 +0200, Jens Axboe <[email protected]> wrote:
> On Fri, Oct 26 2007, Herbert Xu wrote:
>> [CRYPTO] tcrypt: Move sg_init_table out of timing loops
>>
>> This patch moves the sg_init_table out of the timing loops for hash
>> algorithms so that it doesn't impact on the speed test results.
>
> Wouldn't it be better to just make sg_init_one() call sg_init_table?
>
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 4571231..ccc55a6 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -202,28 +202,6 @@ static inline void __sg_mark_end(struct scatterlist *sg)
> }
>
> /**
> - * sg_init_one - Initialize a single entry sg list
> - * @sg: SG entry
> - * @buf: Virtual address for IO
> - * @buflen: IO length
> - *
> - * Notes:
> - * This should not be used on a single entry that is part of a larger
> - * table. Use sg_init_table() for that.
> - *
> - **/
> -static inline void sg_init_one(struct scatterlist *sg, const void *buf,
> - unsigned int buflen)
> -{
> - memset(sg, 0, sizeof(*sg));
> -#ifdef CONFIG_DEBUG_SG
> - sg->sg_magic = SG_MAGIC;
> -#endif
> - sg_mark_end(sg, 1);
> - sg_set_buf(sg, buf, buflen);
> -}
> -
> -/**
> * sg_init_table - Initialize SG table
> * @sgl: The SG table
> * @nents: Number of entries in table
> @@ -247,6 +225,24 @@ static inline void sg_init_table(struct scatterlist *sgl, unsigned int nents)
> }
>
> /**
> + * sg_init_one - Initialize a single entry sg list
> + * @sg: SG entry
> + * @buf: Virtual address for IO
> + * @buflen: IO length
> + *
> + * Notes:
> + * This should not be used on a single entry that is part of a larger
> + * table. Use sg_init_table() for that.
> + *
> + **/
> +static inline void sg_init_one(struct scatterlist *sg, const void *buf,
> + unsigned int buflen)
> +{
> + sg_init_table(sg, 1);
> + sg_set_buf(sg, buf, buflen);
> +}
> +
> +/**
> * sg_phys - Return physical address of an sg entry
> * @sg: SG entry
> *
>
Yes please submit this patch. scsi-ml is full of sg_init_one, specially
on the error recovery path.
Thanks
Boaz
On Tue, Oct 30 2007, Boaz Harrosh wrote:
> On Mon, Oct 29 2007 at 22:16 +0200, Jens Axboe <[email protected]> wrote:
> > On Fri, Oct 26 2007, Herbert Xu wrote:
> >> [CRYPTO] tcrypt: Move sg_init_table out of timing loops
> >>
> >> This patch moves the sg_init_table out of the timing loops for hash
> >> algorithms so that it doesn't impact on the speed test results.
> >
> > Wouldn't it be better to just make sg_init_one() call sg_init_table?
> >
> > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> > index 4571231..ccc55a6 100644
> > --- a/include/linux/scatterlist.h
> > +++ b/include/linux/scatterlist.h
> > @@ -202,28 +202,6 @@ static inline void __sg_mark_end(struct scatterlist *sg)
> > }
> >
> > /**
> > - * sg_init_one - Initialize a single entry sg list
> > - * @sg: SG entry
> > - * @buf: Virtual address for IO
> > - * @buflen: IO length
> > - *
> > - * Notes:
> > - * This should not be used on a single entry that is part of a larger
> > - * table. Use sg_init_table() for that.
> > - *
> > - **/
> > -static inline void sg_init_one(struct scatterlist *sg, const void *buf,
> > - unsigned int buflen)
> > -{
> > - memset(sg, 0, sizeof(*sg));
> > -#ifdef CONFIG_DEBUG_SG
> > - sg->sg_magic = SG_MAGIC;
> > -#endif
> > - sg_mark_end(sg, 1);
> > - sg_set_buf(sg, buf, buflen);
> > -}
> > -
> > -/**
> > * sg_init_table - Initialize SG table
> > * @sgl: The SG table
> > * @nents: Number of entries in table
> > @@ -247,6 +225,24 @@ static inline void sg_init_table(struct scatterlist *sgl, unsigned int nents)
> > }
> >
> > /**
> > + * sg_init_one - Initialize a single entry sg list
> > + * @sg: SG entry
> > + * @buf: Virtual address for IO
> > + * @buflen: IO length
> > + *
> > + * Notes:
> > + * This should not be used on a single entry that is part of a larger
> > + * table. Use sg_init_table() for that.
> > + *
> > + **/
> > +static inline void sg_init_one(struct scatterlist *sg, const void *buf,
> > + unsigned int buflen)
> > +{
> > + sg_init_table(sg, 1);
> > + sg_set_buf(sg, buf, buflen);
> > +}
> > +
> > +/**
> > * sg_phys - Return physical address of an sg entry
> > * @sg: SG entry
> > *
> >
> Yes please submit this patch. scsi-ml is full of sg_init_one, specially
> on the error recovery path.
Will do.
--
Jens Axboe
On Tue, Oct 30, 2007 at 06:50:58AM +0100, Jens Axboe wrote:
>
> How so? The reason you changed it to sg_init_table() + sg_set_buf() is
> exactly because sg_init_one() didn't properly init the entry (as they
> name promised).
For one of the cases yes but the other one repeatedly calls
sg_init_one on the same sg entry while we really only need
to initialise it once and call sg_set_buf afterwards.
Normally this is irrelevant but the loops in question are
trying to estimate the speed of the algorithms so it's good
to exclude as much noise from them as possible.
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
On Tue, Oct 30 2007, Herbert Xu wrote:
> On Tue, Oct 30, 2007 at 06:50:58AM +0100, Jens Axboe wrote:
> >
> > How so? The reason you changed it to sg_init_table() + sg_set_buf() is
> > exactly because sg_init_one() didn't properly init the entry (as they
> > name promised).
>
> For one of the cases yes but the other one repeatedly calls
> sg_init_one on the same sg entry while we really only need
> to initialise it once and call sg_set_buf afterwards.
>
> Normally this is irrelevant but the loops in question are
> trying to estimate the speed of the algorithms so it's good
> to exclude as much noise from them as possible.
Ah OK, I was referring to the replacement mentioned above.
--
Jens Axboe