All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuri Tikhonov <yur@emcraft.com>
To: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: Detlev Zundel <dzu@denx.de>, Ilya Yanok <yanok@emcraft.com>,
	linux-kernel@vger.kernel.org, Milton Miller <miltonm@bga.com>,
	linuxppc-dev@ozlabs.org, Wolfgang Denk <wd@denx.de>
Subject: Re[2]: [PATCH] fork_init: fix division by zero
Date: Wed, 10 Dec 2008 13:01:13 +0300	[thread overview]
Message-ID: <503391615.20081210130113@emcraft.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0812100944080.25126@vixen.sonytel.be>

=0D=0A Hello Geert,

On Wednesday, December 10, 2008 you wrote:
> On Tue, 9 Dec 2008, Yuri Tikhonov wrote:
>> The following patch fixes divide-by-zero error for the
>> cases of really big PAGE_SIZEs (e.g. 256KB on ppc44x).
>> Support for such big page sizes on 44x is not present in the
>> current kernel yet, but coming soon.
>>=20
>> Also this patch fixes the comment for the max_threads
>> settings, as this didn't match the things actually done
>> in the code.
>>=20
>> Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
>> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
>> ---
>>  kernel/fork.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>=20
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 2a372a0..b0ac2fb 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -181,10 +181,14 @@ void __init fork_init(unsigned long mempages)
>> =20
>>       /*
>>        * The default maximum number of threads is set to a safe
>> -      * value: the thread structures can take up at most half
>> -      * of memory.
>> +      * value: the thread structures can take up at most
>> +      * (1/8) part of memory.
>>        */
>> +#if (8 * THREAD_SIZE) > PAGE_SIZE
>>       max_threads =3D mempages / (8 * THREAD_SIZE / PAGE_SIZE);
>> +#else
>> +     max_threads =3D mempages * PAGE_SIZE / (8 * THREAD_SIZE);
>                       ^^^^^^^^^^^^^^^^^^^^
>> +#endif

> Can't this overflow, e.g. on 32-bit machines with HIGHMEM?

 The multiplier here is not PAGE_SIZE, but [PAGE_SIZE / (8 *=20
THREAD_SIZE)], and this value is expected to be rather small (2, 4, or=20
so).

 Furthermore, due to the #if/#endif construction multiplication is=20
used only with rather big PAGE_SIZE values, and the bigger page size=20
is then the smaller 'mempages' is.

 So, for example, when running with PAGE_SIZE=3D256KB, THREAD_SIZE=3D8KB,=
=20
on 32-bit 440spe-based machine with 4GB RAM installed, here we have:

 max_threads =3D (4G/256K) * (256K / 8 * 8K) =3D 16384 * 4 =3D 65536.

 And the overflow will have a place only in case of very very big=20
sizes of RAM: >=3D 256TB:

 max_threads =3D (256T / 256K) * (256K / 8 * 8K) =3D 0x4000.0000 * 4.

 But I don't think that with 256TB RAM installed this code will be=20
the only place of problems :)

 Regards, Yuri

 --
 Yuri Tikhonov, Senior Software Engineer
 Emcraft Systems, www.emcraft.com

WARNING: multiple messages have this Message-ID (diff)
From: Yuri Tikhonov <yur@emcraft.com>
To: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Detlev Zundel <dzu@denx.de>, Wolfgang Denk <wd@denx.de>,
	Milton Miller <miltonm@bga.com>, Ilya Yanok <yanok@emcraft.com>
Subject: Re[2]: [PATCH] fork_init: fix division by zero
Date: Wed, 10 Dec 2008 13:01:13 +0300	[thread overview]
Message-ID: <503391615.20081210130113@emcraft.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0812100944080.25126@vixen.sonytel.be>


 Hello Geert,

On Wednesday, December 10, 2008 you wrote:
> On Tue, 9 Dec 2008, Yuri Tikhonov wrote:
>> The following patch fixes divide-by-zero error for the
>> cases of really big PAGE_SIZEs (e.g. 256KB on ppc44x).
>> Support for such big page sizes on 44x is not present in the
>> current kernel yet, but coming soon.
>> 
>> Also this patch fixes the comment for the max_threads
>> settings, as this didn't match the things actually done
>> in the code.
>> 
>> Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
>> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
>> ---
>>  kernel/fork.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 2a372a0..b0ac2fb 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -181,10 +181,14 @@ void __init fork_init(unsigned long mempages)
>>  
>>       /*
>>        * The default maximum number of threads is set to a safe
>> -      * value: the thread structures can take up at most half
>> -      * of memory.
>> +      * value: the thread structures can take up at most
>> +      * (1/8) part of memory.
>>        */
>> +#if (8 * THREAD_SIZE) > PAGE_SIZE
>>       max_threads = mempages / (8 * THREAD_SIZE / PAGE_SIZE);
>> +#else
>> +     max_threads = mempages * PAGE_SIZE / (8 * THREAD_SIZE);
>                       ^^^^^^^^^^^^^^^^^^^^
>> +#endif

> Can't this overflow, e.g. on 32-bit machines with HIGHMEM?

 The multiplier here is not PAGE_SIZE, but [PAGE_SIZE / (8 * 
THREAD_SIZE)], and this value is expected to be rather small (2, 4, or 
so).

 Furthermore, due to the #if/#endif construction multiplication is 
used only with rather big PAGE_SIZE values, and the bigger page size 
is then the smaller 'mempages' is.

 So, for example, when running with PAGE_SIZE=256KB, THREAD_SIZE=8KB, 
on 32-bit 440spe-based machine with 4GB RAM installed, here we have:

 max_threads = (4G/256K) * (256K / 8 * 8K) = 16384 * 4 = 65536.

 And the overflow will have a place only in case of very very big 
sizes of RAM: >= 256TB:

 max_threads = (256T / 256K) * (256K / 8 * 8K) = 0x4000.0000 * 4.

 But I don't think that with 256TB RAM installed this code will be 
the only place of problems :)

 Regards, Yuri

 --
 Yuri Tikhonov, Senior Software Engineer
 Emcraft Systems, www.emcraft.com


  reply	other threads:[~2008-12-10 10:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-09 17:44 [PATCH] fork_init: fix division by zero Yuri Tikhonov
2008-12-09 17:44 ` Yuri Tikhonov
2008-12-10  8:44 ` Geert Uytterhoeven
2008-12-10  8:44   ` Geert Uytterhoeven
2008-12-10 10:01   ` Yuri Tikhonov [this message]
2008-12-10 10:01     ` Re[2]: " Yuri Tikhonov
2008-12-10 10:17     ` Al Viro
2008-12-10 10:17       ` Al Viro
2008-12-10 10:29       ` Re[2]: " Yuri Tikhonov
2008-12-10 10:29         ` Yuri Tikhonov
2008-12-10 13:06         ` David Howells
2008-12-10 13:06           ` David Howells
2008-12-10 13:09           ` David Howells
2008-12-10 13:09             ` David Howells
2008-12-10 13:15           ` Geert Uytterhoeven
2008-12-10 13:15             ` Geert Uytterhoeven
2008-12-10 13:25           ` Re[4]: " Yuri Tikhonov
2008-12-10 13:25             ` Yuri Tikhonov
2008-12-10 21:50           ` Re[2]: " Paul Mackerras
2008-12-10 21:50             ` Paul Mackerras
2008-12-10 17:25         ` Scott Wood
2008-12-10 17:25           ` Scott Wood
2008-12-10 17:56           ` Re[2]: " Yuri Tikhonov
2008-12-10 17:56             ` Yuri Tikhonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503391615.20081210130113@emcraft.com \
    --to=yur@emcraft.com \
    --cc=Geert.Uytterhoeven@sonycom.com \
    --cc=dzu@denx.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=miltonm@bga.com \
    --cc=wd@denx.de \
    --cc=yanok@emcraft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.