Re: [PATCH 07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
On Tue, Dec 02, 2014 at 03:28:17PM -0500, George Spelvin wrote:
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -51,9 +51,9 @@ struct prng_context {
unsigned char rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
- u32 rand_read_pos; /* Offset into rand_data[] */
+ unsigned char rand_read_pos;/* Offset into rand_data[] */
u8 please. Also, not sure if this helps much, as I think the padding
will just get you back to word alignment on each of these.
I noticed the unsigned char vs u8 issue, but didn't make the change
as I didn't think the readailility improvement was worth the code churn.
You just sent a 17 patch series of clean up and were worried about code churn
converting an unsigned char to a u8?
But I'd be happy to clean that up, too!
Should I convert all the buffers and function prototypes, or is there
some criterion for distinguishing which gets which? (E.g. buffers are
unsigned char but control variables are u8.)
If you want to sure. u8 probably makes more sense for the buffers here as our
intent is to treat them as an array of byte values.
And actually, you do win. spinlock_t is 16 bits on x86,
and the buffers are all 16 bytes. (80 bytes before my earlier
patches, 48 bytes after.)
So the the structure goes from:
32-bit64-bit Variable
OffsetSizeOffset Size
020 2 spinlock_t prng_lock
216 2 16 unsigned char rand_data[16]
1816 18 16 unsigned char DT[16]
3416 34 16 unsigned char V[16]
502 50 2 (alignemnt)
524 52 4 u32 rand_read_pos
564 56 8 struct crypto_cipher *tfm
604 64 4 u32 flags
68 4 (alignment)
6472 (structure size)
to
32-bit64-bit Variable
OffsetSizeOffset Size
3416 34 16 unsigned char V[16]
501 50 1 u8 rand_read_pos
511 51 1 u8 flags
52 4 (alignment)
524 56 8 struct crypto_cipher *tfm
5664 (structure size)
You still get 4 bytes of alignment padding on x86-64, but given that
the structure has 60 bytes of payload, that's the minimum possible.
You save 6 bytes of variables and 2 bytes of padding on both
32- and 64-bit systems, for a total of 8 bytes, and that's enough
to knock you into a smaller slab object bin on 64-bit.
I forget where I read the terminology, but the most efficient
wat to pack a structure is in an organ pipe configuraiton where
the biggest (in terms of *alignment*) members are on the outside
and the structre and the smaller elements are on the inside.
Putting a 32-bit flags after a 64-bit pointer violates that.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
On Tue, Dec 02, 2014 at 03:43:13AM -0500, George Spelvin wrote:
rand_read_pos is never more than 16, while there's only 1 flag
bit allocated, so we can shrink the context a little.
Signed-off-by: George Spelvin li...@horizon.com
---
crypto/ansi_cprng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
They're also reordered to avoid alignment holes.
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c
index 93ed00f6..f40f54cd 100644
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -51,9 +51,9 @@ struct prng_context {
unsigned char rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
- u32 rand_read_pos; /* Offset into rand_data[] */
+ unsigned char rand_read_pos;/* Offset into rand_data[] */
u8 please. Also, not sure if this helps much, as I think the padding will just
get you back to word alignment on each of these.
+ unsigned char flags;
struct crypto_cipher *tfm;
- u32 flags;
};
static int dbg;
--
2.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/17] crypto: ansi_cprng - Shrink rand_read_pos flags
--- a/crypto/ansi_cprng.c
+++ b/crypto/ansi_cprng.c
@@ -51,9 +51,9 @@ struct prng_context {
unsigned char rand_data[DEFAULT_BLK_SZ];
unsigned char DT[DEFAULT_BLK_SZ];
unsigned char V[DEFAULT_BLK_SZ];
-u32 rand_read_pos; /* Offset into rand_data[] */
+unsigned char rand_read_pos;/* Offset into rand_data[] */
u8 please. Also, not sure if this helps much, as I think the padding
will just get you back to word alignment on each of these.
I noticed the unsigned char vs u8 issue, but didn't make the change
as I didn't think the readailility improvement was worth the code churn.
But I'd be happy to clean that up, too!
Should I convert all the buffers and function prototypes, or is there
some criterion for distinguishing which gets which? (E.g. buffers are
unsigned char but control variables are u8.)
And actually, you do win. spinlock_t is 16 bits on x86,
and the buffers are all 16 bytes. (80 bytes before my earlier
patches, 48 bytes after.)
So the the structure goes from:
32-bit 64-bit Variable
Offset SizeOffset Size
0 20 2 spinlock_t prng_lock
2 16 2 16 unsigned char rand_data[16]
18 16 18 16 unsigned char DT[16]
34 16 34 16 unsigned char V[16]
50 2 50 2 (alignemnt)
52 4 52 4 u32 rand_read_pos
56 4 56 8 struct crypto_cipher *tfm
60 4 64 4 u32 flags
68 4 (alignment)
64 72 (structure size)
to
32-bit 64-bit Variable
Offset SizeOffset Size
34 16 34 16 unsigned char V[16]
50 1 50 1 u8 rand_read_pos
51 1 51 1 u8 flags
52 4 (alignment)
52 4 56 8 struct crypto_cipher *tfm
56 64 (structure size)
You still get 4 bytes of alignment padding on x86-64, but given that
the structure has 60 bytes of payload, that's the minimum possible.
You save 6 bytes of variables and 2 bytes of padding on both
32- and 64-bit systems, for a total of 8 bytes, and that's enough
to knock you into a smaller slab object bin on 64-bit.
I forget where I read the terminology, but the most efficient
wat to pack a structure is in an organ pipe configuraiton where
the biggest (in terms of *alignment*) members are on the outside
and the structre and the smaller elements are on the inside.
Putting a 32-bit flags after a 64-bit pointer violates that.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
