RE: [ros-cvs] CVS Update: reactos

[Hartmut Birr]

Hi,

this patch (or some areas in ntoskrnl) isn't correct. This patch puts always
64kB between the TEB's. This limits the number of threads to 30,000 or less.
I've found this by an other bug in kernel32.

- Hartmut

>   -----Original Message-----
>  From: ros-cvs-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Ge van Geldorp
>  Sent: Tuesday, September 28, 2004 12:49 PM
>  To: ros-cvs(a)reactos.com
>  Subject: [ros-cvs] CVS Update: reactos
>  
>  
>  CVSROOT:	/CVS/ReactOS
>  Module name:	reactos
>  Repository:	reactos/ntoskrnl/ps/
>  Changes by:	gvg(a)mok.osexperts.com	04/09/28 12:49:21
>  
>  Modified files:
>  	reactos/ntoskrnl/ex/: sysinfo.c 
>  	reactos/ntoskrnl/include/internal/: mm.h 
>  	reactos/ntoskrnl/mm/: anonmem.c marea.c 
>  	reactos/ntoskrnl/ps/: process.c 
>  
>  Log message:
>  	NtAllocateVirtualMemory() should return 64k aligned areas
>  
>  _______________________________________________
>  Ros-cvs mailing list
>  Ros-cvs(a)reactos.com
>  http://reactos.com/mailman/listinfo/ros-cvs
>

[Alex Ionescu]

Doesn't one of the Mm functions have a special flag to allocate on 4KB 
alignment, designed for this kind of problem that 64KB alignment does?

Best regards,
Alex Ionescu

Hartmut Birr wrote:

>  Hi,
>
> this patch (or some areas in ntoskrnl) isn't correct. This patch puts always
> 64kB between the TEB's. This limits the number of threads to 30,000 or less.
> I've found this by an other bug in kernel32.
>
> - Hartmut
>
>   
>
> >  -----Original Message-----
> > From: ros-cvs-bounces(a)reactos.com 
> > [mailto:[e-mail]] On Behalf Of Ge van Geldorp
> > Sent: Tuesday, September 28, 2004 12:49 PM
> > To: ros-cvs(a)reactos.com
> > Subject: [ros-cvs] CVS Update: reactos
> >
> >
> > CVSROOT:	/CVS/ReactOS
> > Module name:	reactos
> > Repository:	reactos/ntoskrnl/ps/
> > Changes by:	gvg(a)mok.osexperts.com	04/09/28 12:49:21
> >
> > Modified files:
> > 	reactos/ntoskrnl/ex/: sysinfo.c 
> > 	reactos/ntoskrnl/include/internal/: mm.h 
> > 	reactos/ntoskrnl/mm/: anonmem.c marea.c 
> > 	reactos/ntoskrnl/ps/: process.c 
> >
> > Log message:
> > 	NtAllocateVirtualMemory() should return 64k aligned areas
> >
> > _______________________________________________
> > Ros-cvs mailing list
> > Ros-cvs(a)reactos.com
> > http://reactos.com/mailman/listinfo/ros-cvs
> >
> >     
> >  
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.com
> http://reactos.com:8080/mailman/listinfo/ros-dev
>
>   
>

[Ge van Geldorp]

>   From: Hartmut Birr
>  
>  this patch (or some areas in ntoskrnl) isn't correct. This 
>  patch puts always 64kB between the TEB's. This limits the 
>  number of threads to 30,000 or less. I've found this by an 
>  other bug in kernel32. 
Yes, you're right, that is an unwanted side effect. I think I'll change the
TEB allocation to reserve 64k at a time and then commit 4k for each TEB, so
we can have TEBs spaced tightly together again.

Ge van Geldorp.

[Ge van Geldorp]

>   From: Ge van Geldorp
>  
> >   From: Hartmut Birr
> >  
> >  this patch (or some areas in ntoskrnl) isn't correct. This
> >  patch puts always 64kB between the TEB's. This limits the 
> >  number of threads to 30,000 or less. I've found this by an 
> >  other bug in kernel32. 
>  
>  Yes, you're right, that is an unwanted side effect. I think 
>  I'll change the TEB allocation to reserve 64k at a time and 
>  then commit 4k for each TEB, so we can have TEBs spaced 
>  tightly together again. 
This is now implemented. I've successfully created and destroyed 600000
threads in a single process, so we should be ok I guess.

Ge van Geldorp.

[Hartmut Birr]

Hi,    

I've still more problems with the modifications in the virtual memory
functions. My test case is ctm from rosapps. It shows the cpu time and
memory usage of the processes. While waiting for a keyboard event, ctm
(kernel32) destroys and creates every 100ms a new  thread. It seems, that
the memory (teb, stack, ...) isn't freed. The thread itself is destroyed. Ps
shows always two threads for ctm.   

- Hartmut

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Ge van Geldorp
>  Sent: Friday, October 01, 2004 10:46 PM
>  To: 'ReactOS Development List'
>  Subject: [ros-dev] TEB spacing
>  
>  
> >   From: Ge van Geldorp
> >  
> > >   From: Hartmut Birr
> > >  
> > >  this patch (or some areas in ntoskrnl) isn't correct. This
> > >  patch puts always 64kB between the TEB's. This limits the 
> > >  number of threads to 30,000 or less. I've found this by an 
> > >  other bug in kernel32. 
> >  
> >  Yes, you're right, that is an unwanted side effect. I think 
> >  I'll change the TEB allocation to reserve 64k at a time and 
> >  then commit 4k for each TEB, so we can have TEBs spaced 
> >  tightly together again. 
>  
>  This is now implemented. I've successfully created and 
>  destroyed 600000
>  threads in a single process, so we should be ok I guess.
>  
>  Ge van Geldorp.
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>

[Ge van Geldorp]

>   From: Hartmut Birr
>  
>  I've still more problems with the modifications in the 
>  virtual memory functions. My test case is ctm from rosapps. 
>  It shows the cpu time and memory usage of the processes. 
>  While waiting for a keyboard event, ctm
>  (kernel32) destroys and creates every 100ms a new  thread. It 
>  seems, that the memory (teb, stack, ...) isn't freed. The 
>  thread itself is destroyed. Ps
>  shows always two threads for ctm.    
Ok, I'll dig into it to see what's wrong. Is this causing you major problems
(i.e. should I rollback the changes for now)?

The reason for the changes is that the Windows Installer (instmsiw.exe)
depends on VirtualAlloc() returning 64k aligned blocks. There are other
sources (e.g.
http://blogs.msdn.com/oldnewthing/archive/2003/10/08/55239.aspx) that
indicate that VirtualAlloc really should return 64k aligned blocks.

Ge van Geldorp.

[Ge van Geldorp]

>   From: Ge van Geldorp
>  
> >   From: Hartmut Birr
> >  
> >  I've still more problems with the modifications in the
> >  virtual memory functions. My test case is ctm from rosapps. 
> >  It shows the cpu time and memory usage of the processes. 
> >  While waiting for a keyboard event, ctm
> >  (kernel32) destroys and creates every 100ms a new  thread. It 
> >  seems, that the memory (teb, stack, ...) isn't freed. The 
> >  thread itself is destroyed. Ps
> >  shows always two threads for ctm.    
>  
>  Ok, I'll dig into it to see what's wrong. 
Are you sure the behaviour changed recently? What I see is that the TEBs are
properly freed, but the stack (2Mb of committed memory) isn't. That happens
because the threads don't terminate themselves, but are terminated using
TerminateThread(). If a thread calls ExitThread() then its stack is freed,
but if a thread is ended using TerminateThread(), its stack is not freed.

It still needs to be fixed ofcourse, but I'm wondering what my recent
changes have to do with this.

Ge van Geldorp.

[Hartmut Birr]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Ge van Geldorp
>  Sent: Sunday, October 03, 2004 1:20 AM
>  To: 'ReactOS Development List'
>  Subject: RE: [ros-dev] TEB spacing
>  
>  
> >   From: Ge van Geldorp
> >  
> >  > From: Hartmut Birr
> >  > 
> >  > I've still more problems with the modifications in the virtual 
> >  > memory functions. My test case is ctm from rosapps. It  
>  shows the cpu 
> >   > time and memory usage of the processes.
> > While waiting for  
>  a keyboard 
> > >   event,
> > > ctm
> > >  (kernel32) destroys and creates every 100ms a new  thread. It
> > >  seems, that the memory (teb, stack, ...) isn't freed. The 
> > >  thread itself is destroyed. Ps
> > >  shows always two threads for ctm.    
> >  
> >  Ok, I'll dig into it to see what's wrong. 
>  
>  Are you sure the behaviour changed recently? What I see is 
>  that the TEBs are properly freed, but the stack (2Mb of 
>  committed memory) isn't. That happens because the threads 
>  don't terminate themselves, but are terminated using 
>  TerminateThread(). If a thread calls ExitThread() then its 
>  stack is freed, but if a thread is ended using 
>  TerminateThread(), its stack is not freed.
>  
>  It still needs to be fixed ofcourse, but I'm wondering what 
>  my recent changes have to do with this.
>  
>  Ge van Geldorp.
>  
>   
Hi,

I'm not sure if all of my problems from ctm are related to your changes.
After the first problem I've do more testing with ctm. In the past I've used
one console for buildung ros and another which runs ctm. I've build ros
completly. Currently I've get a bugcheck (my first reported problem which is
now fixed) in the teb allocation routines and now I'see that ctm eats up all
available memory.

- Hartmut

[Ge van Geldorp]

>   From: Hartmut Birr
>  
>  I'm not sure if all of my problems from ctm are related to 
>  your changes. After the first problem I've do more testing 
>  with ctm. In the past I've used one console for buildung ros 
>  and another which runs ctm. I've build ros completly. 
>  Currently I've get a bugcheck (my first reported problem 
>  which is now fixed) in the teb allocation routines and now 
>  I'see that ctm eats up all available memory. 
Still don't understand why this problem didn't show up before, but it should
be fixed now. The user stack is now cleaned up when a thread is
TerminateThread()ed too.

Ge van Geldorp.

[Mike Nordell]

Ge van Geldorp wrote:

>   What I see is that the TEBs are properly freed,
>  but the stack (2Mb of committed memory) isn't. That happens
>  because the threads don't terminate themselves, ... 
While I haven't looked at the application at hand, 2MB _committed_ stack
strikes me as odd. Are the two threads actually created with 1MB committed
stack, are they using the whole megabyte of the usually only reserved
memory, or is the PE header stack members set to unusual values?

Default used to be 1MB of reserved stack, with only one page committed.

/Mike

[Ge van Geldorp]

>   From: Mike Nordell
>  
>  While I haven't looked at the application at hand, 2MB 
>  _committed_ stack strikes me as odd. Are the two threads 
>  actually created with 1MB committed stack, are they using the 
>  whole megabyte of the usually only reserved memory, or is the 
>  PE header stack members set to unusual values?
>  
>  Default used to be 1MB of reserved stack, with only one page 
>  committed. 
The PE header says 2MB of reserved stack, with 4096 bytes committed.
However, our implementation overrides that (lib/rosrtl/thread/stack.c line
40), with a comment "FIXME: no SEH, no guard pages".

Gé van Geldorp.

[Mike Nordell]

GvG wrote:

>   There are other sources (...) that
>  indicate that VirtualAlloc really should return 64k aligned blocks. 
If there is any dubt whatsoever about this, there are not just indications
it should return 64K-aligned memory, it is carved in stone it reserves
64KB-aligned memory. Committing reserved memory is then (obviously)
per-page.

/Mike