[ros-kernel] User mode debugger support, ETHREAD is wrong, and ps\suspend.c is weird! (patch comments requested)

[Vizzini]

On Tue, 2004-02-24 at 23:28, Skywing wrote:
> 1) Our ETHREAD is wrong.  Not only do third party headers and websites
> document it as wrong, but a page hosted on Microsoft.com documents it as
> wrong (!) -- see http://www.microsoft.com/mspress/books/sampchap/4354b.asp.
> In particular we're missing HideFromDebugger.  This is needed for some of
> the Dbgk routines.  Is it OK to just stuff it in...? (also note that
> HasTerminated is a ULONG and not a UCHAR like we have it declared as).

This stuff is eventually going to have to match in order for debugging
tools to work right.  Any code that depends on a wrong ETHREAD is
broken, anyway, and will need to be changed.  Try changing this in your
tree and see what happens.  

I realize this isn't exposed by Microsoft, but it's documented in their
PDBs, so there are apps out there (particularly debuggers) that will
depend on it.  Besides, MS is good about only making compatible changes
to things like this from release to release - adding stuff to the end or
re-purposing fields - they don't often just completely change these
things - so that's even more of an argument to get as close to MS's
version as possible.

> 2) Do we _really_ need SuspendMutex in ps\suspend.c?  It's keeping the new
> PsSuspend*Process and PsResume*Process routines from being useable above
> PASSIVE_LEVEL (ugh).  I actually think that no locking mechanism whatsoever
> is needed and that everything SuspendMutex protects can be replaced with
> InterlockedIncrement/InterlockedDecrement.  Any thoughts on this?  Art
> agreed with me that it could probably be replaced safely but if you
> disagree, now's the chance to say so!

Having looked at this, I think you need some sort of lock here.  I could
be wrong, but 5 minutes of fooling with various interlocked functions
left me unable to eliminate the race conditions.

If you want to change it to a spin lock, that's fine by me, but note
that the file has to be non-pageable for when we start paging the
kernel.

> Also, I'd like general comments on my suspend/resume all threads in a
> process algorithm.  Can anyone more knowledgeable than me about the ReactOS
> Ps tell if they're implemented incorrectly?

Other than the locking issue, the functions look fine to me, but I'm a
bad visual debugger. :-)

 -Vizzini