[PATCH 0/7] MR5907: ntdll: Optimize NtQueryVirtualMemory(MemoryWorkingSetExInformation).
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/psapi/tests/psapi_main.c | 85 +++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 24 deletions(-) diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c index e7e5e5f04e6..515364f156d 100644 --- a/dlls/psapi/tests/psapi_main.c +++ b/dlls/psapi/tests/psapi_main.c @@ -1150,6 +1150,32 @@ free_page: VirtualFree(addr, 0, MEM_RELEASE); } +static void check_working_set_info(PSAPI_WORKING_SET_EX_INFORMATION *info, const char *desc, DWORD expected_valid, + DWORD expected_protection, DWORD expected_shared, BOOL todo) +{ + todo_wine_if(todo) + ok(info->VirtualAttributes.Valid == expected_valid, "%s expected Valid=%lu but got %u\n", + desc, expected_valid, info->VirtualAttributes.Valid); + + todo_wine_if(todo) + ok(info->VirtualAttributes.Win32Protection == expected_protection, "%s expected Win32Protection=%lu but got %u\n", + desc, expected_protection, info->VirtualAttributes.Win32Protection); + + ok(info->VirtualAttributes.Node == 0, "%s expected Node=0 but got %u\n", + desc, info->VirtualAttributes.Node); + ok(info->VirtualAttributes.LargePage == 0, "%s expected LargePage=0 but got %u\n", + desc, info->VirtualAttributes.LargePage); + + ok(info->VirtualAttributes.Shared == expected_shared || broken(!info->VirtualAttributes.Valid) /* w2003 */, + "%s expected Shared=%lu but got %u\n", desc, expected_shared, info->VirtualAttributes.Shared); + if (info->VirtualAttributes.Valid && info->VirtualAttributes.Shared) + ok(info->VirtualAttributes.ShareCount > 0, "%s expected ShareCount > 0 but got %u\n", + desc, info->VirtualAttributes.ShareCount); + else + ok(info->VirtualAttributes.ShareCount == 0, "%s expected ShareCount == 0 but got %u\n", + desc, info->VirtualAttributes.ShareCount); +} + static void check_QueryWorkingSetEx(PVOID addr, const char *desc, DWORD expected_valid, DWORD expected_protection, DWORD expected_shared, BOOL todo) { @@ -1161,32 +1187,13 @@ static void check_QueryWorkingSetEx(PVOID addr, const char *desc, DWORD expected ret = pQueryWorkingSetEx(GetCurrentProcess(), &info, sizeof(info)); ok(ret, "QueryWorkingSetEx failed with %ld\n", GetLastError()); - todo_wine_if(todo) - ok(info.VirtualAttributes.Valid == expected_valid, "%s expected Valid=%lu but got %u\n", - desc, expected_valid, info.VirtualAttributes.Valid); - - todo_wine_if(todo) - ok(info.VirtualAttributes.Win32Protection == expected_protection, "%s expected Win32Protection=%lu but got %u\n", - desc, expected_protection, info.VirtualAttributes.Win32Protection); - - ok(info.VirtualAttributes.Node == 0, "%s expected Node=0 but got %u\n", - desc, info.VirtualAttributes.Node); - ok(info.VirtualAttributes.LargePage == 0, "%s expected LargePage=0 but got %u\n", - desc, info.VirtualAttributes.LargePage); - - ok(info.VirtualAttributes.Shared == expected_shared || broken(!info.VirtualAttributes.Valid) /* w2003 */, - "%s expected Shared=%lu but got %u\n", desc, expected_shared, info.VirtualAttributes.Shared); - if (info.VirtualAttributes.Valid && info.VirtualAttributes.Shared) - ok(info.VirtualAttributes.ShareCount > 0, "%s expected ShareCount > 0 but got %u\n", - desc, info.VirtualAttributes.ShareCount); - else - ok(info.VirtualAttributes.ShareCount == 0, "%s expected ShareCount == 0 but got %u\n", - desc, info.VirtualAttributes.ShareCount); + check_working_set_info(&info, desc, expected_valid, expected_protection, expected_shared, todo); } static void test_QueryWorkingSetEx(void) { - PVOID addr; + PSAPI_WORKING_SET_EX_INFORMATION info[4]; + char *addr, *addr2; DWORD prot; BOOL ret; @@ -1196,7 +1203,7 @@ static void test_QueryWorkingSetEx(void) return; } - addr = GetModuleHandleA(NULL); + addr = (void *)GetModuleHandleA(NULL); check_QueryWorkingSetEx(addr, "exe", 1, PAGE_READONLY, 1, FALSE); ret = VirtualProtect(addr, 0x1000, PAGE_NOACCESS, &prot); @@ -1211,7 +1218,7 @@ static void test_QueryWorkingSetEx(void) ok(ret, "VirtualProtect failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "exe,readonly2", 1, PAGE_READONLY, 1, FALSE); - addr = VirtualAlloc(NULL, 0x1000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + addr = VirtualAlloc(NULL, 0x2000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); ok(addr != NULL, "VirtualAlloc failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "valloc", 0, 0, 0, FALSE); @@ -1232,9 +1239,39 @@ static void test_QueryWorkingSetEx(void) *(volatile char *)addr; check_QueryWorkingSetEx(addr, "valloc,readwrite2", 1, PAGE_READWRITE, 0, FALSE); + addr2 = VirtualAlloc(NULL, 0x2000, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + ok(!!addr2, "VirtualAlloc failed with %ld\n", GetLastError()); + *(addr2 + 0x1000) = 1; + + info[1].VirtualAddress = addr; + info[0].VirtualAddress = addr + 0x1000; + info[3].VirtualAddress = addr2; + info[2].VirtualAddress = addr2 + 0x1000; + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[1] range[1] valid", 1, PAGE_READWRITE, 0, FALSE); + check_working_set_info(&info[0], "[1] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[1] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[1] range[2] valid", 1, PAGE_READWRITE, 0, FALSE); + ret = VirtualFree(addr, 0, MEM_RELEASE); ok(ret, "VirtualFree failed with %ld\n", GetLastError()); check_QueryWorkingSetEx(addr, "valloc,free", FALSE, 0, 0, FALSE); + + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[2] range[1] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[0], "[2] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[2] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[2] range[2] valid", 1, PAGE_READWRITE, 0, FALSE); + + VirtualFree(addr2, 0, MEM_RELEASE); + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, sizeof(info)); + ok(ret, "got error %lu\n", GetLastError()); + check_working_set_info(&info[1], "[3] range[1] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[0], "[3] range[0] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[3], "[3] range[3] invalid", 0, 0, 0, FALSE); + check_working_set_info(&info[2], "[3] range[2] invalid", 0, 0, 0, FALSE); } START_TEST(psapi_main) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 4 +++- dlls/psapi/tests/psapi_main.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 3981905bcd3..077b68f59f7 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5145,6 +5145,8 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, return STATUS_INVALID_INFO_CLASS; } + if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; + #if defined(HAVE_LIBPROCSTAT) { struct procstat *pstat; @@ -5241,7 +5243,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, #endif if (res_len) - *res_len = (UINT_PTR)p - (UINT_PTR)info; + *res_len = len; return STATUS_SUCCESS; } diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c index 515364f156d..92529447afa 100644 --- a/dlls/psapi/tests/psapi_main.c +++ b/dlls/psapi/tests/psapi_main.c @@ -1194,6 +1194,8 @@ static void test_QueryWorkingSetEx(void) { PSAPI_WORKING_SET_EX_INFORMATION info[4]; char *addr, *addr2; + NTSTATUS status; + SIZE_T size; DWORD prot; BOOL ret; @@ -1203,6 +1205,25 @@ static void test_QueryWorkingSetEx(void) return; } + size = 0xdeadbeef; + memset(info, 0, sizeof(info)); + status = pNtQueryVirtualMemory(GetCurrentProcess(), NULL, MemoryWorkingSetExInformation, info, 0, &size); + ok(status == STATUS_INFO_LENGTH_MISMATCH, "got %#lx.\n", status); + ok(size == 0xdeadbeef, "got %Iu.\n", size); + + memset(&info, 0, sizeof(info)); + ret = pQueryWorkingSetEx(GetCurrentProcess(), info, 0); + ok(!ret && GetLastError() == ERROR_BAD_LENGTH, "got ret %d, err %lu.\n", ret, GetLastError()); + + size = 0xdeadbeef; + memset(info, 0, sizeof(info)); + status = pNtQueryVirtualMemory(GetCurrentProcess(), NULL, MemoryWorkingSetExInformation, info, + sizeof(*info) + sizeof(*info) / 2, &size); + ok(!status, "got %#lx.\n", status); + ok(!info->VirtualAttributes.Valid, "got %d.\n", info->VirtualAttributes.Valid); + ok(size == sizeof(*info) /* wow64 */ || size == sizeof(*info) + sizeof(*info) / 2 /* win64 */, + "got %Iu, sizeof(info) %Iu.\n", size, sizeof(info)); + addr = (void *)GetModuleHandleA(NULL); check_QueryWorkingSetEx(addr, "exe", 1, PAGE_READONLY, 1, FALSE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 202 +++++++++++++++++++++----------------- 1 file changed, 111 insertions(+), 91 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 077b68f59f7..1372eea78ae 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5129,15 +5129,117 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY return STATUS_SUCCESS; } +#if defined(HAVE_LIBPROCSTAT) +struct fill_working_set_info_data +{ + struct procstat *pstat; + struct kinfo_proc *kip; + unsigned int vmentry_count; + struct kinfo_vmentry *vmentries; +}; + +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + unsigned int proc_count; + + d->kip = NULL; + d->vmentry_count = 0; + d->vmentries = NULL; + + if ((d->pstat = procstat_open_sysctl())) + d->kip = procstat_getprocs( d->pstat, KERN_PROC_PID, getpid(), &proc_count ); + if (d->kip) + d->vmentries = procstat_getvmmap( d->pstat, d->kip, &d->vmentry_count ); + if (!d->vmentries) + WARN( "couldn't get process vmmap, errno %d\n", errno ); +} + +static void free_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + if (d->vmentries) + procstat_freevmmap( d->pstat, d->vmentries ); + if (d->kip) + procstat_freeprocs( d->pstat, d->kip ); + if (d->pstat) + procstat_close( d->pstat ); +} + +static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, + MEMORY_WORKING_SET_EX_INFORMATION *p ) +{ + struct kinfo_vmentry *entry = NULL; + int i; + + for (i = 0; i < d->vmentry_count; i++) + { + if (d->vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[i].kve_end) + { + entry = &d->vmentries[i]; + break; + } + } + + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; + p->VirtualAttributes.Shared = !is_view_valloc( view ); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); +} +#else +static int pagemap_fd = -2; + +struct fill_working_set_info_data +{ +}; + +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ + if (pagemap_fd != -2) return; + +#ifdef O_CLOEXEC + if ((pagemap_fd = open( "/proc/self/pagemap", O_RDONLY | O_CLOEXEC, 0 )) == -1 && errno == EINVAL) +#endif + pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 ); + + if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" ); + else fcntl(pagemap_fd, F_SETFD, FD_CLOEXEC); /* in case O_CLOEXEC isn't supported */ +} + +static void free_fill_working_set_info_data( struct fill_working_set_info_data *d ) +{ +} + +static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, + MEMORY_WORKING_SET_EX_INFORMATION *p ) +{ + UINT64 pagemap; + + if (pagemap_fd == -1 || + pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + { + /* If we don't have pagemap information, default to invalid. */ + pagemap = 0; + } + + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); + p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); +} +#endif + static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, MEMORY_WORKING_SET_EX_INFORMATION *info, SIZE_T len, SIZE_T *res_len ) { -#if !defined(HAVE_LIBPROCSTAT) - static int pagemap_fd = -2; -#endif + struct fill_working_set_info_data data; MEMORY_WORKING_SET_EX_INFORMATION *p; + struct file_view *view; sigset_t sigset; + BYTE vprot; if (process != NtCurrentProcess()) { @@ -5147,100 +5249,18 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; -#if defined(HAVE_LIBPROCSTAT) - { - struct procstat *pstat; - unsigned int proc_count; - struct kinfo_proc *kip = NULL; - unsigned int vmentry_count = 0; - struct kinfo_vmentry *vmentries = NULL; - - pstat = procstat_open_sysctl(); - if (pstat) - kip = procstat_getprocs( pstat, KERN_PROC_PID, getpid(), &proc_count ); - if (kip) - vmentries = procstat_getvmmap( pstat, kip, &vmentry_count ); - if (vmentries == NULL) - WARN( "couldn't get process vmmap, errno %d\n", errno ); - - server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) - { - int i; - struct kinfo_vmentry *entry = NULL; - BYTE vprot; - struct file_view *view; - - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - if ((view = find_view( p->VirtualAddress, 0 )) && - get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) && - (vprot & VPROT_COMMITTED)) - { - for (i = 0; i < vmentry_count && entry == NULL; i++) - { - if (vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= vmentries[i].kve_end) - entry = &vmentries[i]; - } - - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; - p->VirtualAttributes.Shared = !is_view_valloc( view ); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); - } - } - server_leave_uninterrupted_section( &virtual_mutex, &sigset ); - - if (vmentries) - procstat_freevmmap( pstat, vmentries ); - if (kip) - procstat_freeprocs( pstat, kip ); - if (pstat) - procstat_close( pstat ); - } -#else server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - if (pagemap_fd == -2) - { -#ifdef O_CLOEXEC - if ((pagemap_fd = open( "/proc/self/pagemap", O_RDONLY | O_CLOEXEC, 0 )) == -1 && errno == EINVAL) -#endif - pagemap_fd = open( "/proc/self/pagemap", O_RDONLY, 0 ); - - if (pagemap_fd == -1) WARN( "unable to open /proc/self/pagemap\n" ); - else fcntl(pagemap_fd, F_SETFD, FD_CLOEXEC); /* in case O_CLOEXEC isn't supported */ - } - + init_fill_working_set_info_data( &data ); for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) { - BYTE vprot; - UINT64 pagemap; - struct file_view *view; - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - - if ((view = find_view( p->VirtualAddress, 0 )) && - get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) && - (vprot & VPROT_COMMITTED)) - { - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) - { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; - } - - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); - p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); - } + if ((view = find_view( p->VirtualAddress, 0 )) + && get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) + && (vprot & VPROT_COMMITTED)) + fill_working_set_info( &data, view, vprot, p ); } + free_fill_working_set_info_data( &data ); server_leave_uninterrupted_section( &virtual_mutex, &sigset ); -#endif if (res_len) *res_len = len; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 71 ++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 1372eea78ae..3eca766770c 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5129,6 +5129,12 @@ static unsigned int get_memory_region_info( HANDLE process, LPCVOID addr, MEMORY return STATUS_SUCCESS; } +struct working_set_info_ref +{ + char *addr; + SIZE_T orig_index; +}; + #if defined(HAVE_LIBPROCSTAT) struct fill_working_set_info_data { @@ -5231,13 +5237,24 @@ static void fill_working_set_info( struct fill_working_set_info_data *d, struct } #endif +static int compare_working_set_info_ref( const void *a, const void *b ) +{ + const struct working_set_info_ref *r1 = a, *r2 = b; + + if (r1->addr < r2->addr) return -1; + return r1->addr > r2->addr; +} + static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, MEMORY_WORKING_SET_EX_INFORMATION *info, SIZE_T len, SIZE_T *res_len ) { + struct working_set_info_ref ref_buffer[256], *ref = ref_buffer, *r; struct fill_working_set_info_data data; - MEMORY_WORKING_SET_EX_INFORMATION *p; - struct file_view *view; + char *start, *end; + SIZE_T i, count; + size_t size; + struct file_view *view, *prev_view; sigset_t sigset; BYTE vprot; @@ -5249,17 +5266,55 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, if (len < sizeof(*info)) return STATUS_INFO_LENGTH_MISMATCH; + count = len / sizeof(*info); + + if (count > ARRAY_SIZE(ref_buffer)) ref = malloc( count * sizeof(*ref) ); + for (i = 0; i < count; ++i) + { + ref[i].orig_index = i; + ref[i].addr = ROUND_ADDR( info[i].VirtualAddress, page_mask ); + info[i].VirtualAttributes.Flags = 0; + } + qsort( ref, count, sizeof(*ref), compare_working_set_info_ref ); + start = ref[0].addr; + end = ref[count - 1].addr + page_size; + server_enter_uninterrupted_section( &virtual_mutex, &sigset ); init_fill_working_set_info_data( &data ); - for (p = info; (UINT_PTR)(p + 1) <= (UINT_PTR)info + len; p++) + + view = find_view_range( start, end - start ); + while (view && (char *)view->base > start) + { + prev_view = RB_ENTRY_VALUE( rb_prev( &view->entry ), struct file_view, entry ); + if (!prev_view || (char *)prev_view->base + prev_view->size <= start) break; + view = prev_view; + } + + r = ref; + while (view && (char *)view->base < end) { - memset( &p->VirtualAttributes, 0, sizeof(p->VirtualAttributes) ); - if ((view = find_view( p->VirtualAddress, 0 )) - && get_committed_size( view, p->VirtualAddress, &vprot, VPROT_COMMITTED ) - && (vprot & VPROT_COMMITTED)) - fill_working_set_info( &data, view, vprot, p ); + if (start < (char *)view->base) start = view->base; + while (r != ref + count && r->addr < start) ++r; + while (start != (char *)view->base + view->size && r != ref + count + && r->addr < (char *)view->base + view->size) + { + size = get_committed_size( view, start, &vprot, ~VPROT_WRITEWATCH ); + if (vprot & VPROT_COMMITTED) + { + while (r != ref + count && r->addr < start + size) + { + fill_working_set_info( &data, view, vprot, &info[r->orig_index] ); + ++r; + } + } + start += size; + } + if (r == ref + count) break; + view = RB_ENTRY_VALUE( rb_next( &view->entry ), struct file_view, entry ); } + free_fill_working_set_info_data( &data ); + if (ref != ref_buffer) free( ref ); server_leave_uninterrupted_section( &virtual_mutex, &sigset ); if (res_len) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 3eca766770c..99b0845db44 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -2140,7 +2140,7 @@ done: * Get the size of the committed range with equal masked vprot bytes starting at base. * Also return the protections for the first page. */ -static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vprot, BYTE vprot_mask ) +static SIZE_T get_committed_size( struct file_view *view, void *base, size_t max_size, BYTE *vprot, BYTE vprot_mask ) { SIZE_T offset, size; @@ -2159,7 +2159,7 @@ static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vpro req->offset = offset; if (!wine_server_call( req )) { - size = reply->size; + size = min( reply->size, max_size ); if (reply->committed) { *vprot |= VPROT_COMMITTED; @@ -2171,7 +2171,7 @@ static SIZE_T get_committed_size( struct file_view *view, void *base, BYTE *vpro if (!size || !(vprot_mask & ~VPROT_COMMITTED)) return size; } - else size = view->size - offset; + else size = min( view->size - offset, max_size ); return get_vprot_range_size( base, size, vprot_mask, vprot ); } @@ -4921,7 +4921,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T if ((view = find_view( base, size ))) { /* Make sure all the pages are committed */ - if (get_committed_size( view, base, &vprot, VPROT_COMMITTED ) >= size && (vprot & VPROT_COMMITTED)) + if (get_committed_size( view, base, ~(size_t)0, &vprot, VPROT_COMMITTED ) >= size && (vprot & VPROT_COMMITTED)) { old = get_win32_prot( vprot, view->protect ); status = set_protection( view, base, size, new_prot ); @@ -5043,7 +5043,7 @@ static unsigned int fill_basic_memory_info( const void *addr, MEMORY_BASIC_INFOR BYTE vprot; info->AllocationBase = alloc_base; - info->RegionSize = get_committed_size( view, base, &vprot, ~VPROT_WRITEWATCH ); + info->RegionSize = get_committed_size( view, base, ~(size_t)0, &vprot, ~VPROT_WRITEWATCH ); info->State = (vprot & VPROT_COMMITTED) ? MEM_COMMIT : MEM_RESERVE; info->Protect = (vprot & VPROT_COMMITTED) ? get_win32_prot( vprot, view->protect ) : 0; info->AllocationProtect = get_win32_prot( view->protect, view->protect ); @@ -5298,7 +5298,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, while (start != (char *)view->base + view->size && r != ref + count && r->addr < (char *)view->base + view->size) { - size = get_committed_size( view, start, &vprot, ~VPROT_WRITEWATCH ); + size = get_committed_size( view, start, end - start, &vprot, ~VPROT_WRITEWATCH ); if (vprot & VPROT_COMMITTED) { while (r != ref + count && r->addr < start + size) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 78 +++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 99b0845db44..c69eb8c018e 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5171,26 +5171,33 @@ static void free_fill_working_set_info_data( struct fill_working_set_info_data * } static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, - MEMORY_WORKING_SET_EX_INFORMATION *p ) + struct working_set_info_ref *ref, SIZE_T count, + MEMORY_WORKING_SET_EX_INFORMATION *info ) { - struct kinfo_vmentry *entry = NULL; - int i; + SIZE_T i; + int j; - for (i = 0; i < d->vmentry_count; i++) + for (i = 0; i < count; ++i) { - if (d->vmentries[i].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[i].kve_end) + MEMORY_WORKING_SET_EX_INFORMATION *p = &info[ref[i].orig_index]; + struct kinfo_vmentry *entry = NULL; + + for (j = 0; j < d->vmentry_count; j++) { - entry = &d->vmentries[i]; - break; + if (d->vmentries[j].kve_start <= (ULONG_PTR)p->VirtualAddress && (ULONG_PTR)p->VirtualAddress <= d->vmentries[j].kve_end) + { + entry = &d->vmentries[j]; + break; + } } - } - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; - p->VirtualAttributes.Shared = !is_view_valloc( view ); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && entry && entry->kve_type != KVME_TYPE_SWAP; + p->VirtualAttributes.Shared = !is_view_valloc( view ); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + } } #else static int pagemap_fd = -2; @@ -5217,23 +5224,31 @@ static void free_fill_working_set_info_data( struct fill_working_set_info_data * } static void fill_working_set_info( struct fill_working_set_info_data *d, struct file_view *view, BYTE vprot, - MEMORY_WORKING_SET_EX_INFORMATION *p ) + struct working_set_info_ref *ref, SIZE_T count, + MEMORY_WORKING_SET_EX_INFORMATION *info ) { + MEMORY_WORKING_SET_EX_INFORMATION *p; UINT64 pagemap; + SIZE_T i; - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + for (i = 0; i < count; ++i) { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; - } + p = &info[ref[i].orig_index]; - p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); - p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); - if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) - p->VirtualAttributes.ShareCount = 1; /* FIXME */ - if (p->VirtualAttributes.Valid) - p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + if (pagemap_fd == -1 || + pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + { + /* If we don't have pagemap information, default to invalid. */ + pagemap = 0; + } + + p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); + p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); + if (p->VirtualAttributes.Shared && p->VirtualAttributes.Valid) + p->VirtualAttributes.ShareCount = 1; /* FIXME */ + if (p->VirtualAttributes.Valid) + p->VirtualAttributes.Win32Protection = get_win32_prot( vprot, view->protect ); + } } #endif @@ -5299,15 +5314,14 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, && r->addr < (char *)view->base + view->size) { size = get_committed_size( view, start, end - start, &vprot, ~VPROT_WRITEWATCH ); + start += size; if (vprot & VPROT_COMMITTED) { - while (r != ref + count && r->addr < start + size) - { - fill_working_set_info( &data, view, vprot, &info[r->orig_index] ); - ++r; - } + i = 0; + while (r + i != ref + count && r[i].addr < start) ++i; + fill_working_set_info( &data, view, vprot, r, i, info ); + r += i; } - start += size; } if (r == ref + count) break; view = RB_ENTRY_VALUE( rb_next( &view->entry ), struct file_view, entry ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index c69eb8c018e..3f6af622b77 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -5144,7 +5144,7 @@ struct fill_working_set_info_data struct kinfo_vmentry *vmentries; }; -static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d, char *end ) { unsigned int proc_count; @@ -5204,10 +5204,19 @@ static int pagemap_fd = -2; struct fill_working_set_info_data { + UINT64 pm_buffer[256]; + SIZE_T buffer_start; + ssize_t buffer_len; + SIZE_T end_page; }; -static void init_fill_working_set_info_data( struct fill_working_set_info_data *d ) +static void init_fill_working_set_info_data( struct fill_working_set_info_data *d, char *end ) { + d->buffer_start = 0; + d->buffer_len = 0; + d->end_page = (UINT_PTR)end >> page_shift; + memset( d->pm_buffer, 0, sizeof(d->pm_buffer) ); + if (pagemap_fd != -2) return; #ifdef O_CLOEXEC @@ -5229,18 +5238,31 @@ static void fill_working_set_info( struct fill_working_set_info_data *d, struct { MEMORY_WORKING_SET_EX_INFORMATION *p; UINT64 pagemap; - SIZE_T i; + SIZE_T i, page; + ssize_t len; for (i = 0; i < count; ++i) { + page = (UINT_PTR)ref[i].addr >> page_shift; p = &info[ref[i].orig_index]; - if (pagemap_fd == -1 || - pread( pagemap_fd, &pagemap, sizeof(pagemap), ((UINT_PTR)p->VirtualAddress >> page_shift) * sizeof(pagemap) ) != sizeof(pagemap)) + assert(page >= d->buffer_start); + if (page >= d->buffer_start + d->buffer_len) { - /* If we don't have pagemap information, default to invalid. */ - pagemap = 0; + d->buffer_start = page; + len = min( sizeof(d->pm_buffer), (d->end_page - page) * sizeof(pagemap) ); + if (pagemap_fd != -1) + { + d->buffer_len = pread( pagemap_fd, d->pm_buffer, len, page * sizeof(pagemap) ); + if (d->buffer_len != len) + { + d->buffer_len = max( d->buffer_len, 0 ); + memset( d->pm_buffer + d->buffer_len / sizeof(pagemap), 0, len - d->buffer_len ); + } + } + d->buffer_len = len / sizeof(pagemap); } + pagemap = d->pm_buffer[page - d->buffer_start]; p->VirtualAttributes.Valid = !(vprot & VPROT_GUARD) && (vprot & 0x0f) && (pagemap >> 63); p->VirtualAttributes.Shared = !is_view_valloc( view ) && ((pagemap >> 61) & 1); @@ -5295,7 +5317,7 @@ static NTSTATUS get_working_set_ex( HANDLE process, LPCVOID addr, end = ref[count - 1].addr + page_size; server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - init_fill_working_set_info_data( &data ); + init_fill_working_set_info_data( &data, end ); view = find_view_range( start, end - start ); while (view && (char *)view->base > start) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5907
It was spotted that QueryWorkingSetEx() takes an extremely long time when runs over big enough ranges. E. g., Dinogen Online performs the memory scan by querying used memory through CEF, which CEF calls QueryWorkingSetEx for all of its memory chunks in ProcessMemoryDump::CountResidentBytes(). That scan takes ~400-500ms now and causes a visible freeze. After the patchset it is 15-20ms. QueryWorkingSetEx() is of course not a most commonly used function, but once used it is typical to scan big memory ranges (or the whole process address space) and not just a few pages. I made a small benchmark program, attaching it and the output after different patches and from Windows all from the same computer. From the benchmark the scan performed in batches is speed up ~50 times and one page requests ~2.5times. [query_working_set.c](/uploads/bf04ff316abcd7b2956c17f0ee96b5a8/query_working_set.c) ``` Windows: ----- anonymous -----. anonymous sequential: 1.7ms anonymous reverse: 1.0ms anonymous each second: 0.5ms anonymous page by page: 27.5ms ----- file -----. file sequential: 2.7ms file reverse: 2.5ms file each second: 1.3ms file page by page: 45.3ms Current git ----- anonymous -----. anonymous sequential: 166.2ms anonymous reverse: 166.3ms anonymous each second: 83.7ms anonymous page by page: 197.6ms ----- file -----. file sequential: 161.5ms file reverse: 161.4ms file each second: 81.1ms file page by page: 191.2ms After "ntdll: Factor OS-specific parts out of get_working_set_ex()." ----- anonymous -----. anonymous sequential: 166.3ms anonymous reverse: 166.5ms anonymous each second: 83.5ms anonymous page by page: 199.5ms ----- file -----. file sequential: 163.3ms file reverse: 163.9ms file each second: 81.9ms file page by page: 192.9ms After "ntdll: Iterate views instead of requested addresses in get_working_set_ex()." ----- anonymous -----. anonymous sequential: 42.7ms anonymous reverse: 42.5ms anonymous each second: 21.9ms anonymous page by page: 198.7ms ----- file -----. file sequential: 40.1ms file reverse: 40.9ms file each second: 20.1ms file page by page: 197.3ms After "ntdll: Limit vprot scan range to the needed interval in get_working_set_ex()." ----- anonymous -----. anonymous sequential: 42.5ms anonymous reverse: 42.9ms anonymous each second: 21.1ms anonymous page by page: 72.9ms ----- file -----. file sequential: 40.4ms file reverse: 40.1ms file each second: 20.0ms 008c:err:winediag:is_broken_driver Broken NVIDIA RandR detected, falling back to RandR 1.0. Please consider using the Nouveau driver instead. file page by page: 70.2ms After ntdll: Buffer pagemap reads in fill_working_set_info(). ----- anonymous -----. anonymous sequential: 3.5ms anonymous reverse: 3.2ms anonymous each second: 1.8ms anonymous page by page: 74.7ms ----- file -----. file sequential: 2.4ms file reverse: 2.3ms file each second: 1.4ms file page by page: 71.6ms ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5907#note_74001
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)