Re: [ros-dev] [ros-diffs] [dgorbachev] 43333: Fix GCC 4.1.3 warning.

[Dmitry Gorbachev]

Hi,

>   This code has multiple synchronization issues, such as
> spinning on a
>  test-for-spin-lock, which is non-atomic... might as well get rid of
>  the spinlock. 
That synchronization code is borrowed from KdpSerialDebugPrint. Please
tell what are issues, I will fix then in both places.

>   Also, you should probably be using a worker thread
I have read that "because the pool of system worker threads is a
limited resource, WorkItem and WorkItemEx routines can be used only
for operations that take a short period of time. If one of these
routines runs for too long (if it contains an indefinite loop, for
example), the system can deadlock. Therefore, if a driver requires
long periods of delayed processing, it should instead call
PsCreateSystemThread to create its own system thread." So I am not
sure, whether it is better to use a worker thread or a dedicated
thread.

>   instead of consuming system resources for a dedicated
> thread and
>  mucking with priorities. 
The thread is created when booting into "ReactOS (Log file)".

>   Why are you using this bad/broken API? 
What is wrong with that API?

This is what GCC generates:

        lock addl       %eax, _KdpFreeBytes

>   Why are you doing an exchange without even checking
> the result? 
It is not needed.

>   Just use InterlockedAdd 
Unfortunately, there is no such function. :)

>   (but you're not accessing KdpFreeBytes safely
>  elsewhere, so again, you're just wasting time doing bad
>  synchronziation). 
Otherwise, if addition will be interrupted in between, some output can
become lost. This is worse then a bug above, which can cause
repeating. On a non-multiprocessor system, there seems to be no other
problems.