[RANDOM] Move two variables to read_mostly section to save memory

[RANDOM] Move two variables to read_mostly section to save memory

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

While examining vmlinux namelist on i686, I noticed :

c0581300 D random_table
c0581480 d input_pool
c0581580 d random_read_wakeup_thresh
c0581584 d random_write_wakeup_thresh
c0581600 d blocking_pool

That means that the two integers random_read_wakeup_thresh and 
random_write_wakeup_thresh use a full cache entry (128 bytes).

Moving them to read_mostly section can shrinks vmlinux by 120 bytes.

# size vmlinux*
    text    data     bss     dec     hex filename
4835553  450210  610304 5896067  59f783 vmlinux.after_patch
4835553  450330  610304 5896187  59f7fb vmlinux.before_patch

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

[-- Attachment #2: random_read_mostly.patch --]
[-- Type: text/plain, Size: 762 bytes --]

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5fee056..af48e86 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -256,14 +256,14 @@
  * The minimum number of bits of entropy before we wake up a read on
  * /dev/random.  Should be enough to do a significant reseed.
  */
-static int random_read_wakeup_thresh = 64;
+static int random_read_wakeup_thresh __read_mostly = 64;
 
 /*
  * If the entropy count falls under this number of bits, then we
  * should wake up processes which are selecting or polling on write
  * access to /dev/random.
  */
-static int random_write_wakeup_thresh = 128;
+static int random_write_wakeup_thresh __read_mostly = 128;
 
 /*
  * When the input pool goes over trickle_thresh, start dropping most

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Adrian Bunk]

On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
> While examining vmlinux namelist on i686, I noticed :
>
> c0581300 D random_table
> c0581480 d input_pool
> c0581580 d random_read_wakeup_thresh
> c0581584 d random_write_wakeup_thresh
> c0581600 d blocking_pool
>
> That means that the two integers random_read_wakeup_thresh and 
> random_write_wakeup_thresh use a full cache entry (128 bytes).
>
> Moving them to read_mostly section can shrinks vmlinux by 120 bytes.
>
> # size vmlinux*
>    text    data     bss     dec     hex filename
> 4835553  450210  610304 5896067  59f783 vmlinux.after_patch
> 4835553  450330  610304 5896187  59f7fb vmlinux.before_patch
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 5fee056..af48e86 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -256,14 +256,14 @@
>   * The minimum number of bits of entropy before we wake up a read on
>   * /dev/random.  Should be enough to do a significant reseed.
>   */
> -static int random_read_wakeup_thresh = 64;
> +static int random_read_wakeup_thresh __read_mostly = 64;
>  
>  /*
>   * If the entropy count falls under this number of bits, then we
>   * should wake up processes which are selecting or polling on write
>   * access to /dev/random.
>   */
> -static int random_write_wakeup_thresh = 128;
> +static int random_write_wakeup_thresh __read_mostly = 128;

Please never ever do such ugly and unmaintainable micro-optimizations in 
the code unless you can show a measurable performance improvement of the 
kernel.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Eric Dumazet]

Adrian Bunk a écrit :
> On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
>> While examining vmlinux namelist on i686, I noticed :
>>
>> c0581300 D random_table
>> c0581480 d input_pool
>> c0581580 d random_read_wakeup_thresh
>> c0581584 d random_write_wakeup_thresh
>> c0581600 d blocking_pool
>>
>> That means that the two integers random_read_wakeup_thresh and 
>> random_write_wakeup_thresh use a full cache entry (128 bytes).
>>
>> Moving them to read_mostly section can shrinks vmlinux by 120 bytes.
>>
>> # size vmlinux*
>>    text    data     bss     dec     hex filename
>> 4835553  450210  610304 5896067  59f783 vmlinux.after_patch
>> 4835553  450330  610304 5896187  59f7fb vmlinux.before_patch
>>
>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> 
>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>> index 5fee056..af48e86 100644
>> --- a/drivers/char/random.c
>> +++ b/drivers/char/random.c
>> @@ -256,14 +256,14 @@
>>   * The minimum number of bits of entropy before we wake up a read on
>>   * /dev/random.  Should be enough to do a significant reseed.
>>   */
>> -static int random_read_wakeup_thresh = 64;
>> +static int random_read_wakeup_thresh __read_mostly = 64;
>>  
>>  /*
>>   * If the entropy count falls under this number of bits, then we
>>   * should wake up processes which are selecting or polling on write
>>   * access to /dev/random.
>>   */
>> -static int random_write_wakeup_thresh = 128;
>> +static int random_write_wakeup_thresh __read_mostly = 128;
> 
> Please never ever do such ugly and unmaintainable micro-optimizations in 
> the code unless you can show a measurable performance improvement of the 
> kernel.

You seem to to be confused between speed micro-otimizations and memory 
savings. This patch has nothing to do about a speed optimization. Here, no 
tradeoff justify a "measurable performance improvement" study.

I copied this patch to you because your recent proposal to remove read_mostly 
from linux kernel.

Only you find read_mostly ugly and unmaintanable. I find it way more usefull 
than "static" attributes.

I find 120 bytes is a measurable gain, thank you.

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Adrian Bunk]

On Sun, Dec 16, 2007 at 03:44:37PM +0100, Eric Dumazet wrote:
> Adrian Bunk a écrit :
>> On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
>>> While examining vmlinux namelist on i686, I noticed :
>>>
>>> c0581300 D random_table
>>> c0581480 d input_pool
>>> c0581580 d random_read_wakeup_thresh
>>> c0581584 d random_write_wakeup_thresh
>>> c0581600 d blocking_pool
>>>
>>> That means that the two integers random_read_wakeup_thresh and 
>>> random_write_wakeup_thresh use a full cache entry (128 bytes).
>>>
>>> Moving them to read_mostly section can shrinks vmlinux by 120 bytes.
>>>
>>> # size vmlinux*
>>>    text    data     bss     dec     hex filename
>>> 4835553  450210  610304 5896067  59f783 vmlinux.after_patch
>>> 4835553  450330  610304 5896187  59f7fb vmlinux.before_patch
>>>
>>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
>>
>>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>>> index 5fee056..af48e86 100644
>>> --- a/drivers/char/random.c
>>> +++ b/drivers/char/random.c
>>> @@ -256,14 +256,14 @@
>>>   * The minimum number of bits of entropy before we wake up a read on
>>>   * /dev/random.  Should be enough to do a significant reseed.
>>>   */
>>> -static int random_read_wakeup_thresh = 64;
>>> +static int random_read_wakeup_thresh __read_mostly = 64;
>>>   /*
>>>   * If the entropy count falls under this number of bits, then we
>>>   * should wake up processes which are selecting or polling on write
>>>   * access to /dev/random.
>>>   */
>>> -static int random_write_wakeup_thresh = 128;
>>> +static int random_write_wakeup_thresh __read_mostly = 128;
>>
>> Please never ever do such ugly and unmaintainable micro-optimizations in 
>> the code unless you can show a measurable performance improvement of the 
>> kernel.
>
> You seem to to be confused between speed micro-otimizations and memory 
> savings. This patch has nothing to do about a speed optimization. Here, no 
> tradeoff justify a "measurable performance improvement" study.
> 
> I copied this patch to you because your recent proposal to remove 
> read_mostly from linux kernel.
>
> Only you find read_mostly ugly and unmaintanable. I find it way more 
> usefull than "static" attributes.
>
> I find 120 bytes is a measurable gain, thank you.


I am well aware that your patch is about space saving and not speed
improvement.

But trying to save space this way is simply not maintainable.

And it's trivial to see that your patch actually makes the code _bigger_ 
for all people who try hard to get their kernel small and use 
CONFIG_SYSCTL=n - funnily your patch has exactly the problem I described 
as drawback of __read_mostly in the thread you are referring to...


And even more funny, with gcc 4.2 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y your 
patch doesn't seem to make any space difference - are you using an older 
compiler or even worse CONFIG_CC_OPTIMIZE_FOR_SIZE=n for being able to 
see any space difference?

In both cases your code uglification would be even more pointless...


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Eric Dumazet]

Adrian Bunk a écrit :
> On Sun, Dec 16, 2007 at 03:44:37PM +0100, Eric Dumazet wrote:
>> Adrian Bunk a écrit :
>>> On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
>>>> While examining vmlinux namelist on i686, I noticed :
>>>>
>>>> c0581300 D random_table
>>>> c0581480 d input_pool
>>>> c0581580 d random_read_wakeup_thresh
>>>> c0581584 d random_write_wakeup_thresh
>>>> c0581600 d blocking_pool
>>>>
>>>> That means that the two integers random_read_wakeup_thresh and 
>>>> random_write_wakeup_thresh use a full cache entry (128 bytes).
>>>>
>>>> Moving them to read_mostly section can shrinks vmlinux by 120 bytes.
>>>>
>>>> # size vmlinux*
>>>>    text    data     bss     dec     hex filename
>>>> 4835553  450210  610304 5896067  59f783 vmlinux.after_patch
>>>> 4835553  450330  610304 5896187  59f7fb vmlinux.before_patch
>>>>
>>>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
>>>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>>>> index 5fee056..af48e86 100644
>>>> --- a/drivers/char/random.c
>>>> +++ b/drivers/char/random.c
>>>> @@ -256,14 +256,14 @@
>>>>   * The minimum number of bits of entropy before we wake up a read on
>>>>   * /dev/random.  Should be enough to do a significant reseed.
>>>>   */
>>>> -static int random_read_wakeup_thresh = 64;
>>>> +static int random_read_wakeup_thresh __read_mostly = 64;
>>>>   /*
>>>>   * If the entropy count falls under this number of bits, then we
>>>>   * should wake up processes which are selecting or polling on write
>>>>   * access to /dev/random.
>>>>   */
>>>> -static int random_write_wakeup_thresh = 128;
>>>> +static int random_write_wakeup_thresh __read_mostly = 128;
>>> Please never ever do such ugly and unmaintainable micro-optimizations in 
>>> the code unless you can show a measurable performance improvement of the 
>>> kernel.
>> You seem to to be confused between speed micro-otimizations and memory 
>> savings. This patch has nothing to do about a speed optimization. Here, no 
>> tradeoff justify a "measurable performance improvement" study.
>>
>> I copied this patch to you because your recent proposal to remove 
>> read_mostly from linux kernel.
>>
>> Only you find read_mostly ugly and unmaintanable. I find it way more 
>> usefull than "static" attributes.
>>
>> I find 120 bytes is a measurable gain, thank you.
> 
> 
> I am well aware that your patch is about space saving and not speed
> improvement.
> 
> But trying to save space this way is simply not maintainable.
> 
> And it's trivial to see that your patch actually makes the code _bigger_ 
> for all people who try hard to get their kernel small and use 
> CONFIG_SYSCTL=n - funnily your patch has exactly the problem I described 
> as drawback of __read_mostly in the thread you are referring to...
> 
> 
> And even more funny, with gcc 4.2 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y your 
> patch doesn't seem to make any space difference - are you using an older 
> compiler or even worse CONFIG_CC_OPTIMIZE_FOR_SIZE=n for being able to 
> see any space difference?
> 
> In both cases your code uglification would be even more pointless...
> 

I believe that CONFIG_SMP is uglification for you Adrian, but still I am glad 
linux have it.

If your CONFIG_SYSCTL=n is really that important for you, why dont you define 
a new qualifier that can indeed mark some variables as :

const if CONFIG_SYSCTL=n
read_mostly if CONFIG_SYCTL=y

This way you can keep compiler optimizations for your CONFIG_SYCTL=n, while 
many people like me can still continue to optimize their kernel.

Seeing so many sysctl already read_mostly in kernel, I wonder why you NACK my 
patch, while it's easy to share your concerns with other people and find a 
solution.

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Adrian Bunk]

On Sun, Dec 16, 2007 at 06:42:57PM +0100, Eric Dumazet wrote:
> Adrian Bunk a écrit :
>...
>> And even more funny, with gcc 4.2 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y your 
>> patch doesn't seem to make any space difference - are you using an older 
>> compiler or even worse CONFIG_CC_OPTIMIZE_FOR_SIZE=n for being able to see 
>> any space difference?
>>
>> In both cases your code uglification would be even more pointless...
>>
>
> I believe that CONFIG_SMP is uglification for you Adrian, but still I am 
> glad linux have it.
>
> If your CONFIG_SYSCTL=n is really that important for you, why dont you 
> define a new qualifier that can indeed mark some variables as :
>
> const if CONFIG_SYSCTL=n
> read_mostly if CONFIG_SYCTL=y
>
> This way you can keep compiler optimizations for your CONFIG_SYCTL=n, while 
> many people like me can still continue to optimize their kernel.
>
> Seeing so many sysctl already read_mostly in kernel, I wonder why you NACK 
> my patch, while it's easy to share your concerns with other people and find 
> a solution.

You omitted an answer to my main important point...

Let me ask it in a more simple way:

Do you see any space difference at all with gcc 4.2 and 
CONFIG_CC_OPTIMIZE_FOR_SIZE=y ?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Eric Dumazet]

Adrian Bunk a écrit :
> On Sun, Dec 16, 2007 at 06:42:57PM +0100, Eric Dumazet wrote:
>> Adrian Bunk a écrit :
>> ...
>>> And even more funny, with gcc 4.2 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y your 
>>> patch doesn't seem to make any space difference - are you using an older 
>>> compiler or even worse CONFIG_CC_OPTIMIZE_FOR_SIZE=n for being able to see 
>>> any space difference?
>>>
>>> In both cases your code uglification would be even more pointless...
>>>
>> I believe that CONFIG_SMP is uglification for you Adrian, but still I am 
>> glad linux have it.
>>
>> If your CONFIG_SYSCTL=n is really that important for you, why dont you 
>> define a new qualifier that can indeed mark some variables as :
>>
>> const if CONFIG_SYSCTL=n
>> read_mostly if CONFIG_SYCTL=y
>>
>> This way you can keep compiler optimizations for your CONFIG_SYCTL=n, while 
>> many people like me can still continue to optimize their kernel.
>>
>> Seeing so many sysctl already read_mostly in kernel, I wonder why you NACK 
>> my patch, while it's easy to share your concerns with other people and find 
>> a solution.
> 
> You omitted an answer to my main important point...
> 
> Let me ask it in a more simple way:
> 
> Do you see any space difference at all with gcc 4.2 and 
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y ?
> 


I am using gcc-4.2.1

CONFIG_CC_OPTIMIZE_FOR_SIZE=y makes no difference for me.

$ make defconfig
$ egrep "OPTIMIZE_FOR_SIZE|CONFIG_SMP" .config
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_SMP=y
$ make vmlinux
$ nm -v vmlinux|grep -4 random_read_wakeup_thresh
c057a02c d excluded_id_list
c057a100 d zero_bdi
c057a180 D random_table
c057a300 d input_pool
c057a400 d random_read_wakeup_thresh
c057a404 d random_write_wakeup_thresh
    <SAME HOLE HERE>
c057a480 d blocking_pool
c057a580 d nonblocking_pool
c057a680 d rekey_work

After my patch, I still gain 120 bytes.

Please realize that most people now build their kernels with CONFIG_SMP=y, or 
use a distro one (with CONFIG_SMP=y as well)

Your CONFIG_SYSCTL point is valid and should be addressed by a separate patch set.

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Adrian Bunk]

On Sun, Dec 16, 2007 at 07:38:14PM +0100, Eric Dumazet wrote:
>
> I am using gcc-4.2.1
>
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y makes no difference for me.
>
> $ make defconfig
> $ egrep "OPTIMIZE_FOR_SIZE|CONFIG_SMP" .config
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y
> CONFIG_SMP=y
> $ make vmlinux
> $ nm -v vmlinux|grep -4 random_read_wakeup_thresh
> c057a02c d excluded_id_list
> c057a100 d zero_bdi
> c057a180 D random_table
> c057a300 d input_pool
> c057a400 d random_read_wakeup_thresh
> c057a404 d random_write_wakeup_thresh
>    <SAME HOLE HERE>
> c057a480 d blocking_pool
> c057a580 d nonblocking_pool
> c057a680 d rekey_work
>
> After my patch, I still gain 120 bytes.
>
> Please realize that most people now build their kernels with CONFIG_SMP=y, 
> or use a distro one (with CONFIG_SMP=y as well)
> 
> Your CONFIG_SYSCTL point is valid and should be addressed by a separate patch set.

How many patches do you want to send only for saving 120 bytes in some 
configurations (and even not the ones people who really care about the 
kernel size usually use)?

And most C files in the kernel would allow you to save more than
120 bytes if you don't mind how tihe source code looks like and e.g. 
don't mind turning it into an #ifdef mess.

If you care about the kernel size, you could e.g. help in fighting 
removals of unused EXPORT_SYMBOL's through Andrew instead - these are 
space saving patches that neither make the C code look worse nor can 
have negative impact on the generated code.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Matt Mackall]

On Sun, Dec 16, 2007 at 07:38:14PM +0100, Eric Dumazet wrote:
> Adrian Bunk a ??crit :
> >On Sun, Dec 16, 2007 at 06:42:57PM +0100, Eric Dumazet wrote:
> >>Adrian Bunk a ??crit :
> >>...
> >>>And even more funny, with gcc 4.2 and CONFIG_CC_OPTIMIZE_FOR_SIZE=y your 
> >>>patch doesn't seem to make any space difference - are you using an older 
> >>>compiler or even worse CONFIG_CC_OPTIMIZE_FOR_SIZE=n for being able to 
> >>>see any space difference?
> >>>
> >>>In both cases your code uglification would be even more pointless...
> >>>
> >>I believe that CONFIG_SMP is uglification for you Adrian, but still I am 
> >>glad linux have it.
> >>
> >>If your CONFIG_SYSCTL=n is really that important for you, why dont you 
> >>define a new qualifier that can indeed mark some variables as :
> >>
> >>const if CONFIG_SYSCTL=n
> >>read_mostly if CONFIG_SYCTL=y
> >>
> >>This way you can keep compiler optimizations for your CONFIG_SYCTL=n, 
> >>while many people like me can still continue to optimize their kernel.
> >>
> >>Seeing so many sysctl already read_mostly in kernel, I wonder why you 
> >>NACK my patch, while it's easy to share your concerns with other people 
> >>and find a solution.
> >
> >You omitted an answer to my main important point...
> >
> >Let me ask it in a more simple way:
> >
> >Do you see any space difference at all with gcc 4.2 and 
> >CONFIG_CC_OPTIMIZE_FOR_SIZE=y ?
> >
> 
> 
> I am using gcc-4.2.1
> 
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y makes no difference for me.
> 
> $ make defconfig
> $ egrep "OPTIMIZE_FOR_SIZE|CONFIG_SMP" .config
> CONFIG_CC_OPTIMIZE_FOR_SIZE=y
> CONFIG_SMP=y
> $ make vmlinux
> $ nm -v vmlinux|grep -4 random_read_wakeup_thresh
> c057a02c d excluded_id_list
> c057a100 d zero_bdi
> c057a180 D random_table
> c057a300 d input_pool
> c057a400 d random_read_wakeup_thresh
> c057a404 d random_write_wakeup_thresh
>    <SAME HOLE HERE>
> c057a480 d blocking_pool
> c057a580 d nonblocking_pool
> c057a680 d rekey_work
> 
> After my patch, I still gain 120 bytes.

Well there's really no point arguing about this. We've found the cause
of the hole (good), but moving other things around to magically fix it
is the wrong thing to do.

I'll queue a patch to remove the big ugly alignment.

Automatically detecting these sorts of holes in the kernel image would
be a useful thing to do. In a couple instances, I've spotted much
larger ones.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Matt Mackall]

On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
> While examining vmlinux namelist on i686, I noticed :
> 
> c0581300 D random_table
> c0581480 d input_pool
> c0581580 d random_read_wakeup_thresh
> c0581584 d random_write_wakeup_thresh
> c0581600 d blocking_pool
> 
> That means that the two integers random_read_wakeup_thresh and 
> random_write_wakeup_thresh use a full cache entry (128 bytes).

But why did that happen?

Probably because of this:

        spinlock_t lock ____cacheline_aligned_in_smp;

in struct entropy_store.

And that comes from here:

http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commitdiff;h=47b54fbff358a1d5ee4738cec8a53a08bead72e4;hp=ce334bb8f0f084112dcfe96214cacfa0afba7e10

So we could save more memory by just dropping that alignment.

The trick is to improve the scalability without it. Currently, for
every 10 bytes read, we hash the whole output pool and do three
feedback cycles, each grabbing the lock briefly and releasing it. We
also need to grab the lock every 128 bytes to do some accounting. So
we do 40 locks every 128 output bytes! Also, the output pool itself
gets bounced back and forth like mad too.

I'm actually in the middle of redoing some patches that will reduce
this to one lock per 10 bytes, or 14 locks per 128 bytes. 

But we can't do much better than that without some fairly serious
restructuring. Like switching to SHA-512, which would take us to one
lock for every 32 output bytes, or 5 locks per 128 bytes with accounting.

We could also switch to per-cpu output pools for /dev/urandom, which
would add 128 bytes of data per CPU, but would eliminate the lock
contention and the pool cacheline bouncing. Is it worth the added
complexity? Probably not.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [RANDOM] Move two variables to read_mostly section to save memory

[Eric Dumazet]

Matt Mackall a écrit :
> On Sun, Dec 16, 2007 at 12:45:01PM +0100, Eric Dumazet wrote:
>> While examining vmlinux namelist on i686, I noticed :
>>
>> c0581300 D random_table
>> c0581480 d input_pool
>> c0581580 d random_read_wakeup_thresh
>> c0581584 d random_write_wakeup_thresh
>> c0581600 d blocking_pool
>>
>> That means that the two integers random_read_wakeup_thresh and 
>> random_write_wakeup_thresh use a full cache entry (128 bytes).
> 
> But why did that happen?
> 
> Probably because of this:
> 
>         spinlock_t lock ____cacheline_aligned_in_smp;
> 
> in struct entropy_store.
> 
> And that comes from here:
> 
> http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commitdiff;h=47b54fbff358a1d5ee4738cec8a53a08bead72e4;hp=ce334bb8f0f084112dcfe96214cacfa0afba7e10
> 
> So we could save more memory by just dropping that alignment.
> 
> The trick is to improve the scalability without it. Currently, for
> every 10 bytes read, we hash the whole output pool and do three
> feedback cycles, each grabbing the lock briefly and releasing it. We
> also need to grab the lock every 128 bytes to do some accounting. So
> we do 40 locks every 128 output bytes! Also, the output pool itself
> gets bounced back and forth like mad too.
> 
> I'm actually in the middle of redoing some patches that will reduce
> this to one lock per 10 bytes, or 14 locks per 128 bytes. 
> 
> But we can't do much better than that without some fairly serious
> restructuring. Like switching to SHA-512, which would take us to one
> lock for every 32 output bytes, or 5 locks per 128 bytes with accounting.
> 
> We could also switch to per-cpu output pools for /dev/urandom, which
> would add 128 bytes of data per CPU, but would eliminate the lock
> contention and the pool cacheline bouncing. Is it worth the added
> complexity? Probably not.
> 

Yes, this reminds me the prefetch_range(r->pool, wordmask); is wrong (should 
be prefetch_range(r->pool, wordmask*4) , so I am not sure how it could help 
David to get better performance.... :(