Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread David Rientjes
On Wed, 25 Feb 2015, Heinrich Schuchardt wrote: > > I disagree strongly: it's better to first do low-risk > > cleanups, then do any fix and other changes. > > > > This approach has four advantages: > > > > - it makes the bug fix more apparent, in the context of > > an already cleaned up

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread Heinrich Schuchardt
On 25.02.2015 11:17, Ingo Molnar wrote: > > * David Rientjes wrote: > >> The problem is with the structure of your patchset. You >> want three patches. There's one bugfix patch, a >> preparation patch, and a feature patch. The bugfix patch >> should come first so that it can be applied,

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread Ingo Molnar
* David Rientjes wrote: > The problem is with the structure of your patchset. You > want three patches. There's one bugfix patch, a > preparation patch, and a feature patch. The bugfix patch > should come first so that it can be applied, possibly, to > stable kernels and doesn't depend

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread Heinrich Schuchardt
On 25.02.2015 11:17, Ingo Molnar wrote: * David Rientjes rient...@google.com wrote: The problem is with the structure of your patchset. You want three patches. There's one bugfix patch, a preparation patch, and a feature patch. The bugfix patch should come first so that it can be

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread David Rientjes
On Wed, 25 Feb 2015, Heinrich Schuchardt wrote: I disagree strongly: it's better to first do low-risk cleanups, then do any fix and other changes. This approach has four advantages: - it makes the bug fix more apparent, in the context of an already cleaned up code.

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-25 Thread Ingo Molnar
* David Rientjes rient...@google.com wrote: The problem is with the structure of your patchset. You want three patches. There's one bugfix patch, a preparation patch, and a feature patch. The bugfix patch should come first so that it can be applied, possibly, to stable kernels and

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
On 24.02.2015 23:16, David Rientjes wrote: > On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: > >>> I'm afraid I don't understand this. The intent of the patch is to >>> separate the max_threads logic into a new function, correct? If that's >>> true, then I don't understand why UINT_MAX is

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread David Rientjes
On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: > > I'm afraid I don't understand this. The intent of the patch is to > > separate the max_threads logic into a new function, correct? If that's > > true, then I don't understand why UINT_MAX is being introduced into this > > path and passed to

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
On 24.02.2015 22:03, David Rientjes wrote: > On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: > >> diff --git a/init/main.c b/init/main.c >> index 61b99376..21394ec 100644 >> --- a/init/main.c >> +++ b/init/main.c >> @@ -94,7 +94,7 @@ >> static int kernel_init(void *); >> >> extern void

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread David Rientjes
On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: > diff --git a/init/main.c b/init/main.c > index 61b99376..21394ec 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -94,7 +94,7 @@ > static int kernel_init(void *); > > extern void init_IRQ(void); > -extern void fork_init(unsigned long); >

[PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
PAGE_SIZE is not guaranteed to be equal to or less than 8 times the THREAD_SIZE. E.g. architecture hexagon may have page size 1M and thread size 4096. This would lead to a division by zero in the calculation of max_threads. With this patch the buggy code is moved to a separate function

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
On 24.02.2015 22:03, David Rientjes wrote: On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: diff --git a/init/main.c b/init/main.c index 61b99376..21394ec 100644 --- a/init/main.c +++ b/init/main.c @@ -94,7 +94,7 @@ static int kernel_init(void *); extern void init_IRQ(void); -extern

[PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
PAGE_SIZE is not guaranteed to be equal to or less than 8 times the THREAD_SIZE. E.g. architecture hexagon may have page size 1M and thread size 4096. This would lead to a division by zero in the calculation of max_threads. With this patch the buggy code is moved to a separate function

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread David Rientjes
On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: diff --git a/init/main.c b/init/main.c index 61b99376..21394ec 100644 --- a/init/main.c +++ b/init/main.c @@ -94,7 +94,7 @@ static int kernel_init(void *); extern void init_IRQ(void); -extern void fork_init(unsigned long); +extern void

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread Heinrich Schuchardt
On 24.02.2015 23:16, David Rientjes wrote: On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: I'm afraid I don't understand this. The intent of the patch is to separate the max_threads logic into a new function, correct? If that's true, then I don't understand why UINT_MAX is being

Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads

2015-02-24 Thread David Rientjes
On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: I'm afraid I don't understand this. The intent of the patch is to separate the max_threads logic into a new function, correct? If that's true, then I don't understand why UINT_MAX is being introduced into this path and passed to the new