Re: [1/1] kernel32: Implemented basic NUMA functions to replace the stubs
2013/7/30 Chris Moeller : > > On Jul 29, 2013, at 11:22 AM, Matteo Bruni wrote: > >> 2013/7/29 Chris Moeller : >>> without looking very deeply into the CRT source code. >> >> You're talking about MS source code, right? That makes you tainted for >> contributing to Wine at >> the very least for C runtime-related code :( >> If you haven't looked into any other source code, this patch might be >> okay (barring other issues, I haven't read the patch at all). At least >> http://wiki.winehq.org/DeveloperFaq#head-fed5011434f62ae1a88baebfb8193a37ea795101 >> seems to suggest so. > > I did not look at any CRT source code to determine how to implement the > functionality of this patch. I only looked at CRT source code to determine > why the CRT was crashing in the first place. The HighestNode function popping > a warning in my wine logs should have been hint enough to discover that > function was stubbed. > > I actually determined how these functions worked by testing their behavior > against a working system, and also with the aid of another developer, who > pointed out a handy Win32 function for retrieving a suitable value for the > free memory check. That's probably fine, although as a general rule you should stay as far as possible from Microsoft's source code. Cheers, Matteo.
Re: [1/1] kernel32: Implemented basic NUMA functions to replace the stubs
Hi Chris. Welcome to Wine. > Main changes from HEAD: > 1) Implements GetNumaHighestNodeNumber which simulates a typical single node > system. > 2) Implements GetNumaNodeProcessorMask which returns the global processor > affinity mask for node 0, otherwise returns an error. > 3) Implements GetNumaAvailableMemoryNode which returns available physical > memory for node 0, otherwise returns an error. The hacks from the codeweavers Wine tree are not in the main tree, because they are hacks. Wine has either a stub or proper implementation with tests. For your changes: - SetLastError(0) is wrong. lasterror is only valid on a failure. (Exceptions are very very rare) - Most implementations in kernel32.dll use functions provides by ntdll.dll With grep, you will find numa support in dlls/ntdll/nt.c - Make the patches as small as possible: Only one implementation per Patch. - Please add tests and run them on Windows. Ask here, if you need help. (Wine has a testbot for approved developer) Thanks for improving Wine. -- By by ... Detlef
Re: [1/1] kernel32: Implemented basic NUMA functions to replace the stubs
2013/7/29 Chris Moeller : > without looking very deeply into the CRT source code. You're talking about MS source code, right? That makes you tainted for contributing to Wine at the very least for C runtime-related code :( If you haven't looked into any other source code, this patch might be okay (barring other issues, I haven't read the patch at all). At least http://wiki.winehq.org/DeveloperFaq#head-fed5011434f62ae1a88baebfb8193a37ea795101 seems to suggest so.
[1/1] kernel32: Implemented basic NUMA functions to replace the stubs
Main changes from HEAD:
1) Implements GetNumaHighestNodeNumber which simulates a typical single node
system.
2) Implements GetNumaNodeProcessorMask which returns the global processor
affinity mask for node 0, otherwise returns an error.
3) Implements GetNumaAvailableMemoryNode which returns available physical
memory for node 0, otherwise returns an error.
This much of the behavior was observed based on a single processor (4 cores)
Windows 7 system. These functions are utilized by the synchronization and
threading primitives incorporated in the MSVC 2012 and newer C and C++ runtime
libraries. concrt.h/Concurrency::critical_section, which is used by both
std::mutex and std::thread, will throw an exception the moment the object is
locked if GetNumaHighestNodeNumber fails, and there are at least a few examples
of software which don't bother to catch this exception and thus crash, such as
the alpha version of Cube World, and certain versions of the foo_wave_seekbar
component for the foobar2000 audio player. The associated scheduling code
appears to make use of GetNumaNodeProcessorMask as well to control how many
threads are allowed to spin up at once, or so I guess without looking very
deeply into the CRT source code.
From a7ae257aed539fc72f0bc37c85030f8abc181389 Mon Sep 17 00:00:00 2001
From: Chris Moeller
Date: Sun, 28 Jul 2013 16:23:05 -0700
Subject: kernel32: Implemented basic NUMA functions to replace the stubs
---
dlls/kernel32/process.c | 42 +-
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c
index 6ce43d8..2513be6 100644
--- a/dlls/kernel32/process.c
+++ b/dlls/kernel32/process.c
@@ -3849,9 +3849,10 @@ HRESULT WINAPI
RegisterApplicationRecoveryCallback(APPLICATION_RECOVERY_CALLBACK
*/
BOOL WINAPI GetNumaHighestNodeNumber(PULONG highestnode)
{
-FIXME("(%p): stub\n", highestnode);
-SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
-return FALSE;
+TRACE("(%p)\n", highestnode);
+*highestnode = 0;
+SetLastError(0);
+return TRUE;
}
/**
@@ -3859,9 +3860,20 @@ BOOL WINAPI GetNumaHighestNodeNumber(PULONG highestnode)
*/
BOOL WINAPI GetNumaNodeProcessorMask(UCHAR node, PULONGLONG mask)
{
-FIXME("(%c %p): stub\n", node, mask);
-SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
-return FALSE;
+TRACE("(%c %p)\n", node, mask);
+if (node == 0)
+{
+DWORD system_mask;
+GetProcessAffinityMask(GetCurrentProcess(), NULL, &system_mask);
+*mask = system_mask;
+SetLastError(0);
+return TRUE;
+}
+else
+{
+SetLastError(ERROR_INVALID_PARAMETER);
+return FALSE;
+}
}
/**
@@ -3869,9 +3881,21 @@ BOOL WINAPI GetNumaNodeProcessorMask(UCHAR node,
PULONGLONG mask)
*/
BOOL WINAPI GetNumaAvailableMemoryNode(UCHAR node, PULONGLONG available_bytes)
{
-FIXME("(%c %p): stub\n", node, available_bytes);
-SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
-return FALSE;
+TRACE("(%c %p)\n", node, available_bytes);
+if (node == 0)
+{
+MEMORYSTATUSEX msx;
+msx.dwLength = sizeof(msx);
+GlobalMemoryStatusEx(&msx);
+*available_bytes = msx.ullAvailPhys;
+SetLastError(0);
+return TRUE;
+}
+else
+{
+SetLastError(ERROR_INVALID_PARAMETER);
+return FALSE;
+}
}
/**
--
1.7.10.2 (Apple Git-33)
