[PATCH 0/3] MR3124: Move periodic registry save out of wineserver
The problem is that registry save is a very heavy operation (scheduled each 30sec in wineserver) during which server doesn't process any requests and the whole prefix is stalled for the duration of the operation. For some reference, the process takes from 50-100ms here up to 1-1.5sec with default initial registry (after some registry modifications which trigger actual registry flush), depending on the filesystem type and state (as huge time may be spent in file close / rename). With the same registry after this patchset the server part (flush_key returning the whole registry data) is taking ~4-5mcs, measured from the client side so that already includes data transfer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3124
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/registry.c | 290 ++++++++++++++++++++++++++++++++++++- server/protocol.def | 14 +- server/registry.c | 124 +++++++++++++--- 3 files changed, 399 insertions(+), 29 deletions(-) diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index a24d6e7c267..b63c7a66925 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -27,6 +27,8 @@ #include <stdarg.h> #include <string.h> +#include <unistd.h> +#include <errno.h> #include "ntstatus.h" #define WIN32_NO_STATUS @@ -68,6 +70,248 @@ NTSTATUS open_hkcu_key( const char *path, HANDLE *key ) return NtCreateKey( key, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL ); } +/* dump a Unicode string with proper escaping */ +int dump_strW( const WCHAR *str, data_size_t len, FILE *f, const char escape[2] ) +{ + static const char escapes[32] = ".......abtnvfr.............e...."; + char buffer[256]; + char *pos = buffer; + int count = 0; + + for (len /= sizeof(WCHAR); len; str++, len--) + { + if (pos > buffer + sizeof(buffer) - 8) + { + fwrite( buffer, pos - buffer, 1, f ); + count += pos - buffer; + pos = buffer; + } + if (*str > 127) /* hex escape */ + { + if (len > 1 && str[1] < 128 && isxdigit( (char)str[1] )) + pos += sprintf( pos, "\\x%04x", *str ); + else + pos += sprintf( pos, "\\x%x", *str ); + continue; + } + if (*str < 32) /* octal or C escape */ + { + if (!*str && len == 1) continue; /* do not output terminating NULL */ + if (escapes[*str] != '.') + pos += sprintf( pos, "\\%c", escapes[*str] ); + else if (len > 1 && str[1] >= '0' && str[1] <= '7') + pos += sprintf( pos, "\\%03o", *str ); + else + pos += sprintf( pos, "\\%o", *str ); + continue; + } + if (*str == '\\' || *str == escape[0] || *str == escape[1]) *pos++ = '\\'; + *pos++ = *str; + } + fwrite( buffer, pos - buffer, 1, f ); + count += pos - buffer; + return count; +} + +struct saved_key +{ + data_size_t namelen; + WCHAR *name; + data_size_t classlen; + WCHAR *class; + int value_count; + int subkey_count; + unsigned int is_symlink; + timeout_t modif; + struct saved_key *parent; +}; + +/* read serialized key data */ +static char *fill_saved_key( struct saved_key *key, struct saved_key *parent, char *data ) +{ + key->parent = parent; + key->namelen = *(data_size_t *)data; + data += sizeof(data_size_t); + key->name = (WCHAR *)data; + data += key->namelen; + key->classlen = *(data_size_t *)data; + data += sizeof(data_size_t); + key->class = (WCHAR *)data; + data += key->classlen; + key->value_count = *(int *)data; + data += sizeof(int); + key->subkey_count = *(int *)data; + data += sizeof(int); + key->is_symlink = *(unsigned int *)data; + data += sizeof(unsigned int); + key->modif = *(timeout_t *)data; + data += sizeof(timeout_t); + + return data; +} + +/* dump serialized key full path */ +static char *dump_parents( char *data, FILE *f, int count ) +{ + data_size_t len; + WCHAR *name; + + len = *(data_size_t *)data; + data += sizeof(data_size_t); + name = (WCHAR *)data; + data += len; + + if (count > 1) + { + data = dump_parents( data, f, count - 1); + fprintf( f, "\\\\" ); + } + dump_strW( name, len, f, "[]" ); + return data; +} + +/* dump the full path of a key */ +static void dump_path( const struct saved_key *key, const struct saved_key *base, FILE *f ) +{ + if (key->parent && key->parent != base) + { + dump_path( key->parent, base, f ); + fprintf( f, "\\\\" ); + } + dump_strW( key->name, key->namelen, f, "[]" ); +} + +/* dump a value to a text file */ +static char *dump_value( char *data, FILE *f ) +{ + unsigned int i, dw; + int count; + data_size_t namelen, valuelen; + char *valuedata; + WCHAR *name; + unsigned int type; + + namelen = *(data_size_t *)data; + data += sizeof(data_size_t); + name = (WCHAR *)data; + data += namelen; + type = *(unsigned int *)data; + data += sizeof(unsigned int); + valuelen = *(data_size_t *)data; + data += sizeof(data_size_t); + valuedata = data; + data += valuelen; + + if (namelen) + { + fputc( '\"', f ); + count = 1 + dump_strW( name, namelen, f, "\"\"" ); + count += fprintf( f, "\"=" ); + } + else count = fprintf( f, "@=" ); + + switch(type) + { + case REG_SZ: + case REG_EXPAND_SZ: + case REG_MULTI_SZ: + /* only output properly terminated strings in string format */ + if (valuelen < sizeof(WCHAR)) break; + if (valuelen % sizeof(WCHAR)) break; + if (((WCHAR *)valuedata)[valuelen / sizeof(WCHAR) - 1]) break; + if (type != REG_SZ) fprintf( f, "str(%x):", type ); + fputc( '\"', f ); + dump_strW( (WCHAR *)valuedata, valuelen, f, "\"\"" ); + fprintf( f, "\"\n" ); + return data; + + case REG_DWORD: + if (valuelen != sizeof(dw)) break; + memcpy( &dw, valuedata, sizeof(dw) ); + fprintf( f, "dword:%08x\n", dw ); + return data; + } + + if (type == REG_BINARY) count += fprintf( f, "hex:" ); + else count += fprintf( f, "hex(%x):", type ); + for (i = 0; i < valuelen; i++) + { + count += fprintf( f, "%02x", *((unsigned char *)valuedata + i) ); + if (i < valuelen-1) + { + fputc( ',', f ); + if (++count > 76) + { + fprintf( f, "\\\n " ); + count = 2; + } + } + } + fputc( '\n', f ); + return data; +} + +/* save a registry key and all its subkeys to a text file */ +static char *save_subkeys( char *data, struct saved_key *parent, struct saved_key *base, FILE *f ) +{ + struct saved_key key; + int i; + + if (!base) base = &key; + data = fill_saved_key( &key, parent, data ); + + /* save key if it has either some values or no subkeys, or needs special options */ + /* keys with no values but subkeys are saved implicitly by saving the subkeys */ + if ((key.value_count > 0) || !key.subkey_count || key.classlen || key.is_symlink) + { + fprintf( f, "\n[" ); + if (parent) dump_path( &key, base, f ); + fprintf( f, "] %u\n", (unsigned int)((key.modif - SECS_1601_TO_1970 * TICKSPERSEC) / TICKSPERSEC) ); + fprintf( f, "#time=%x%08x\n", (unsigned int)(key.modif >> 32), (unsigned int)key.modif ); + if (key.classlen) + { + fprintf( f, "#class=\"" ); + dump_strW( key.class, key.classlen, f, "\"\"" ); + fprintf( f, "\"\n" ); + } + if (key.is_symlink) fputs( "#link\n", f ); + for (i = 0; i < key.value_count; i++) data = dump_value( data, f ); + } + for (i = 0; i < key.subkey_count; i++) data = save_subkeys( data, &key, base, f ); + return data; +} + +/* save a registry branch to a file */ +static void save_all_subkeys( char *data, FILE *f ) +{ + enum prefix_type prefix_type; + int parent_count; + + prefix_type = *(int *)data; + data += sizeof(int); + + parent_count = *(int *)data; + data += sizeof(int); + + fprintf( f, "WINE REGISTRY Version 2\n" ); + fprintf( f, ";; All keys relative to " ); + data = dump_parents( data, f, parent_count ); + fprintf( f, "\n" ); + + switch (prefix_type) + { + case PREFIX_32BIT: + fprintf( f, "\n#arch=win32\n" ); + break; + case PREFIX_64BIT: + fprintf( f, "\n#arch=win64\n" ); + break; + default: + break; + } + save_subkeys( data, NULL, NULL, f ); +} + /****************************************************************************** * NtCreateKey (NTDLL.@) @@ -776,17 +1020,53 @@ NTSTATUS WINAPI NtUnloadKey( OBJECT_ATTRIBUTES *attr ) */ NTSTATUS WINAPI NtSaveKey( HANDLE key, HANDLE file ) { + data_size_t size = 0; unsigned int ret; + char *data = NULL; + int fd, fd2, needs_close = 0; + FILE *f; TRACE( "(%p,%p)\n", key, file ); - SERVER_START_REQ( save_registry ) + while (1) { - req->hkey = wine_server_obj_handle( key ); - req->file = wine_server_obj_handle( file ); - ret = wine_server_call( req ); + SERVER_START_REQ( save_registry ) + { + req->hkey = wine_server_obj_handle( key ); + if (size) wine_server_set_reply( req, data, size ); + ret = wine_server_call( req ); + size = reply->total; + } + SERVER_END_REQ; + + if (!ret) break; + free( data ); + if (ret != STATUS_BUFFER_TOO_SMALL) return ret; + if (!(data = malloc( size ))) + { + ERR( "No memory.\n" ); + return STATUS_NO_MEMORY; + } } - SERVER_END_REQ; + + if ((ret = server_get_unix_fd( file, FILE_WRITE_DATA, &fd, &needs_close, NULL, NULL ))) goto done; + if ((fd2 = dup( fd )) == -1) + { + ret = errno_to_status( errno ); + goto done; + } + if (!(f = fdopen( fd2, "w" ))) + { + close( fd2 ); + ret = errno_to_status( errno ); + goto done; + } + save_all_subkeys( data, f ); + if (fclose(f)) ret = errno_to_status( errno ); + +done: + if (needs_close) close( fd ); + free( data ); return ret; } diff --git a/server/protocol.def b/server/protocol.def index 54059b6b580..2e020984a4d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1897,11 +1897,19 @@ struct process_info @END -/* Save a registry branch to a file */ +/* Return full registry branch non-volatile data for saving */ @REQ(save_registry) - obj_handle_t hkey; /* key to save */ - obj_handle_t file; /* file to save to */ + obj_handle_t hkey; /* key to save */ +(a)REPLY + data_size_t total; /* total length needed for data */ + VARARG(data,bytes); /* registry data */ @END +enum prefix_type +{ + PREFIX_UNKNOWN, + PREFIX_32BIT, + PREFIX_64BIT, +}; /* Add a registry key change notification */ diff --git a/server/registry.c b/server/registry.c index 3fea7dd79a4..ba87e7c79c4 100644 --- a/server/registry.c +++ b/server/registry.c @@ -124,7 +124,7 @@ static struct key *root_key; static const timeout_t ticks_1601_to_1970 = (timeout_t)86400 * (369 * 365 + 89) * TICKS_PER_SEC; static const timeout_t save_period = 30 * -TICKS_PER_SEC; /* delay between periodic saves */ static struct timeout_user *save_timeout_user; /* saving timer */ -static enum prefix_type { PREFIX_UNKNOWN, PREFIX_32BIT, PREFIX_64BIT } prefix_type; +static enum prefix_type prefix_type; static const WCHAR wow6432node[] = {'W','o','w','6','4','3','2','N','o','d','e'}; static const WCHAR symlink_value[] = {'S','y','m','b','o','l','i','c','L','i','n','k','V','a','l','u','e'}; @@ -2021,29 +2021,104 @@ static void save_all_subkeys( struct key *key, FILE *f ) save_subkeys( key, key, f ); } -/* save a registry branch to a file handle */ -static void save_registry( struct key *key, obj_handle_t handle ) +static data_size_t serialize_value( const struct key_value *value, char *buf ) { - struct file *file; - int fd; + data_size_t size; - if (!(file = get_file_obj( current->process, handle, FILE_WRITE_DATA ))) return; - fd = dup( get_file_unix_fd( file ) ); - release_object( file ); - if (fd != -1) + size = sizeof(data_size_t) + value->namelen + sizeof(unsigned int) + sizeof(data_size_t) + value->len; + if (!buf) return size; + + *(data_size_t *)buf = value->namelen; + buf += sizeof(data_size_t); + memcpy( buf, value->name, value->namelen ); + buf += value->namelen; + + *(unsigned int *)buf = value->type; + buf += sizeof(unsigned int); + + *(data_size_t *)buf = value->len; + buf += sizeof(data_size_t); + memcpy( buf, value->data, value->len ); + + return size; +} + +/* save a registry key with subkeys to a buffer */ +static data_size_t serialize_key( const struct key *key, char *buf ) +{ + data_size_t size; + int subkey_count, i; + + if (key->flags & KEY_VOLATILE) return 0; + + size = sizeof(data_size_t) + key->obj.name->len + sizeof(data_size_t) + key->classlen + sizeof(int) + sizeof(int) + + sizeof(unsigned int) + sizeof(timeout_t); + for (i = 0; i <= key->last_value; i++) + size += serialize_value( &key->values[i], buf ? buf + size : NULL ); + subkey_count = 0; + for (i = 0; i <= key->last_subkey; i++) { - FILE *f = fdopen( fd, "w" ); - if (f) - { - save_all_subkeys( key, f ); - if (fclose( f )) file_set_error(); - } - else - { - file_set_error(); - close( fd ); - } + if (key->subkeys[i]->flags & KEY_VOLATILE) continue; + size += serialize_key( key->subkeys[i], buf ? buf + size : NULL ); + ++subkey_count; + } + if (!buf) return size; + + *(data_size_t *)buf = key->obj.name->len; + buf += sizeof(data_size_t); + memcpy( buf, key->obj.name->name, key->obj.name->len ); + buf += key->obj.name->len; + + *(data_size_t *)buf = key->classlen; + buf += sizeof(data_size_t); + memcpy( buf, key->class, key->classlen ); + buf += key->classlen; + + *(int *)buf = key->last_value + 1; + buf += sizeof(int); + + *(int *)buf = subkey_count; + buf += sizeof(int); + + *(unsigned int *)buf = key->flags & KEY_SYMLINK; + buf += sizeof(unsigned int); + + *(timeout_t *)buf = key->modif; + + return size; +} + +/* save registry branch to buffer */ +static data_size_t save_registry( const struct key *key, char *buf ) +{ + int *parent_count = NULL; + const struct key *parent; + data_size_t size; + + size = sizeof(int) + sizeof(int); + if (buf) + { + *(int *)buf = prefix_type; + buf += sizeof(int); + parent_count = (int *)buf; + buf += sizeof(int); + *parent_count = 0; } + + parent = key; + do + { + size += sizeof(data_size_t) + parent->obj.name->len; + if (!buf) continue; + ++*parent_count; + *(data_size_t *)buf = parent->obj.name->len; + buf += sizeof(data_size_t); + memcpy( buf, parent->obj.name->name, parent->obj.name->len ); + buf += parent->obj.name->len; + } while ((parent = get_parent( parent ))); + + size += serialize_key( key, buf ); + return size; } /* save a registry branch to a file */ @@ -2370,6 +2445,7 @@ DECL_HANDLER(unload_registry) DECL_HANDLER(save_registry) { struct key *key; + char *data; if (!thread_single_check_privilege( current, SeBackupPrivilege )) { @@ -2379,7 +2455,13 @@ DECL_HANDLER(save_registry) if ((key = get_hkey_obj( req->hkey, 0 ))) { - save_registry( key, req->file ); + reply->total = save_registry( key, NULL ); + if (reply->total <= get_reply_max_size()) + { + if ((data = set_reply_data_size( reply->total ))) + save_registry( key, data ); + } + else set_error( STATUS_BUFFER_TOO_SMALL ); release_object( key ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3124
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/registry.c | 155 +++++++++++++++++++++++++++++++++++-- server/protocol.def | 12 +++ server/registry.c | 94 ++++++++++++++++++++-- 3 files changed, 247 insertions(+), 14 deletions(-) diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index b63c7a66925..14b3ade3e6d 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -29,6 +29,8 @@ #include <string.h> #include <unistd.h> #include <errno.h> +#include <fcntl.h> +#include <sys/stat.h> #include "ntstatus.h" #define WIN32_NO_STATUS @@ -282,8 +284,9 @@ static char *save_subkeys( char *data, struct saved_key *parent, struct saved_ke } /* save a registry branch to a file */ -static void save_all_subkeys( char *data, FILE *f ) +static char *save_all_subkeys( char *data, FILE *f ) { + /* Output registry format should match server/registry.c:save_all_subkeys(). */ enum prefix_type prefix_type; int parent_count; @@ -309,7 +312,7 @@ static void save_all_subkeys( char *data, FILE *f ) default: break; } - save_subkeys( data, NULL, NULL, f ); + return save_subkeys( data, NULL, NULL, f ); } @@ -901,22 +904,162 @@ NTSTATUS WINAPI NtNotifyChangeKey( HANDLE key, HANDLE event, PIO_APC_ROUTINE apc io, filter, subtree, buffer, length, async ); } +/* acquire mutex for registry flush operation */ +static HANDLE get_key_flush_mutex(void) +{ + WCHAR bufferW[256]; + UNICODE_STRING name = {.Buffer = bufferW}; + OBJECT_ATTRIBUTES attr; + char buffer[256]; + HANDLE mutex; + + snprintf( buffer, ARRAY_SIZE(buffer), "\\Sessions\\%u\\BaseNamedObjects\\__wine_regkey_flush", + (int)NtCurrentTeb()->Peb->SessionId ); + name.Length = name.MaximumLength = (strlen(buffer) + 1) * sizeof(WCHAR); + ascii_to_unicode( bufferW, buffer, name.Length / sizeof(WCHAR) ); + + InitializeObjectAttributes( &attr, &name, OBJ_OPENIF, NULL, NULL ); + if (NtCreateMutant( &mutex, MUTEX_ALL_ACCESS, &attr, FALSE ) < 0) return NULL; + NtWaitForSingleObject( mutex, FALSE, NULL ); + return mutex; +} + +/* release registry flush mutex */ +static void release_key_flush_mutex( HANDLE mutex ) +{ + NtReleaseMutant( mutex, NULL ); + NtClose( mutex ); +} + +/* save registry branch to Wine regsitry storage file */ +static NTSTATUS save_registry_branch( char **data ) +{ + static const char temp_fn[] = "savereg.tmp"; + char *file_name, *path = NULL, *tmp = NULL; + int file_name_len, path_len, fd; + struct stat st; + NTSTATUS ret; + FILE *f; + + file_name_len = *(int *)*data; + *data += sizeof(int); + file_name = *data; + *data += file_name_len; + + path_len = strlen( config_dir ) + 1 + file_name_len + 1; + if (!(path = malloc( path_len ))) return STATUS_NO_MEMORY; + sprintf( path, "%s/%s", config_dir, file_name ); + + if ((fd = open( path, O_WRONLY )) != -1) + { + /* if file is not a regular file or has multiple links or is accessed + * via symbolic links, write directly into it; otherwise use a temp file */ + if (!lstat( path, &st ) && (!S_ISREG(st.st_mode) || st.st_nlink > 1)) + { + ftruncate( fd, 0 ); + goto save; + } + close( fd ); + } + + /* create a temp file in the same directory */ + if (!(tmp = malloc( strlen( config_dir ) + 1 + strlen( temp_fn ) + 1 ))) + { + ret = STATUS_NO_MEMORY; + goto done; + } + sprintf( tmp, "%s/%s", config_dir, temp_fn ); + + if ((fd = open( tmp, O_CREAT | O_EXCL | O_WRONLY, 0666 )) == -1) + { + ret = errno_to_status( errno ); + goto done; + } + +save: + if (!(f = fdopen( fd, "w" ))) + { + ret = errno_to_status( errno ); + if (tmp) unlink( tmp ); + close( fd ); + goto done; + } + + *data = save_all_subkeys( *data, f ); + + ret = fclose( f ) ? errno_to_status( errno ) : STATUS_SUCCESS; + if (tmp) + { + if (!ret && rename( tmp, path )) ret = errno_to_status( errno ); + if (ret) unlink( tmp ); + } + +done: + free( tmp ); + free( path ); + return ret; +} /****************************************************************************** * NtFlushKey (NTDLL.@) */ NTSTATUS WINAPI NtFlushKey( HANDLE key ) { + abstime_t timestamp_counter; + data_size_t size = 0; unsigned int ret; + char *data = NULL, *curr_data; + HANDLE mutex; + int i, branch_count, branch; TRACE( "key=%p\n", key ); - SERVER_START_REQ( flush_key ) + mutex = get_key_flush_mutex(); + + while (1) { - req->hkey = wine_server_obj_handle( key ); - ret = wine_server_call( req ); + SERVER_START_REQ( flush_key ) + { + req->hkey = wine_server_obj_handle( key ); + if (size) wine_server_set_reply( req, data, size ); + ret = wine_server_call( req ); + size = reply->total; + branch_count = reply->branch_count; + timestamp_counter = reply->timestamp_counter; + } + SERVER_END_REQ; + + if (ret != STATUS_BUFFER_TOO_SMALL) break; + free( data ); + if (!(data = malloc( size ))) + { + ERR( "No memory.\n" ); + ret = STATUS_NO_MEMORY; + goto done; + } } - SERVER_END_REQ; + if (ret) goto done; + + curr_data = data; + for (i = 0; i < branch_count; ++i) + { + branch = *(int *)curr_data; + curr_data += sizeof(int); + if ((ret = save_registry_branch( &curr_data ))) goto done; + + SERVER_START_REQ( flush_key_done ) + { + req->branch = branch; + req->timestamp_counter = timestamp_counter; + ret = wine_server_call( req ); + } + SERVER_END_REQ; + if (ret) break; + } + +done: + release_key_flush_mutex( mutex ); + free( data ); return ret; } diff --git a/server/protocol.def b/server/protocol.def index 2e020984a4d..955ae674841 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1817,6 +1817,18 @@ struct process_info /* Flush a registry key */ @REQ(flush_key) obj_handle_t hkey; /* handle to the key */ +(a)REPLY + abstime_t timestamp_counter; /* branch last change timestamp counter */ + data_size_t total; /* total length needed for data */ + int branch_count; /* number of registry branches to flush */ + VARARG(data,bytes); /* registry data */ +(a)END + + +/* Clear KEY_DIRTY after key flush */ +(a)REQ(flush_key_done) + abstime_t timestamp_counter; /* timestamp counter returned from flush_key */ + int branch; /* saved registry branch id */ @END diff --git a/server/registry.c b/server/registry.c index ba87e7c79c4..6a668c9146c 100644 --- a/server/registry.c +++ b/server/registry.c @@ -90,6 +90,7 @@ struct key unsigned int flags; /* flags */ timeout_t modif; /* last modification time */ struct list notify_list; /* list of notifications */ + abstime_t timestamp_counter; /* timestamp counter at last change */ }; /* key flags */ @@ -118,6 +119,8 @@ struct key_value #define MAX_NAME_LEN 256 /* max. length of a key name */ #define MAX_VALUE_LEN 16383 /* max. length of a value name */ +static abstime_t change_timestamp_counter; + /* the root of the registry tree */ static struct key *root_key; @@ -708,6 +711,7 @@ static struct key *create_key_object( struct object *parent, const struct unicod key->last_value = -1; key->values = NULL; key->modif = modif; + key->timestamp_counter = 0; list_init( &key->notify_list ); if (options & REG_OPTION_CREATE_LINK) key->flags |= KEY_SYMLINK; @@ -728,23 +732,25 @@ static struct key *create_key_object( struct object *parent, const struct unicod /* mark a key and all its parents as dirty (modified) */ static void make_dirty( struct key *key ) { + ++change_timestamp_counter; while (key) { if (key->flags & (KEY_DIRTY|KEY_VOLATILE)) return; /* nothing to do */ key->flags |= KEY_DIRTY; + key->timestamp_counter = change_timestamp_counter; key = get_parent( key ); } } /* mark a key and all its subkeys as clean (not modified) */ -static void make_clean( struct key *key ) +static void make_clean( struct key *key, abstime_t timestamp_counter ) { int i; if (key->flags & KEY_VOLATILE) return; if (!(key->flags & KEY_DIRTY)) return; - key->flags &= ~KEY_DIRTY; - for (i = 0; i <= key->last_subkey; i++) make_clean( key->subkeys[i] ); + if (key->timestamp_counter <= timestamp_counter) key->flags &= ~KEY_DIRTY; + for (i = 0; i <= key->last_subkey; i++) make_clean( key->subkeys[i], timestamp_counter ); } /* go through all the notifications and send them if necessary */ @@ -2003,6 +2009,7 @@ void init_registry(void) /* save a registry branch to a file */ static void save_all_subkeys( struct key *key, FILE *f ) { + /* Registry format in ntdll/registry.c:save_all_subkeys() should match. */ fprintf( f, "WINE REGISTRY Version 2\n" ); fprintf( f, ";; All keys relative to " ); dump_path( key, NULL, f ); @@ -2191,7 +2198,7 @@ static int save_branch( struct key *key, const char *path ) done: free( tmp ); - if (ret) make_clean( key ); + if (ret) make_clean( key, key->timestamp_counter ); return ret; } @@ -2239,6 +2246,36 @@ static int is_wow64_thread( struct thread *thread ) return (is_machine_64bit( native_machine ) && !is_machine_64bit( thread->process->machine )); } +/* find all the branches inside the specified key or the branch containing the key */ +static void find_branches_for_key( struct key *key, int *branches, int *branch_count ) +{ + struct key *k; + int i; + + *branch_count = 0; + for (i = 0; i < save_branch_count; i++) + { + k = save_branch_info[i].key; + while ((k = get_parent(k))) + { + if (k != key) continue; + branches[(*branch_count)++] = i; + break; + } + } + + if (*branch_count) return; + + do + { + for (i = 0; i < save_branch_count; i++) + { + if(key != save_branch_info[i].key) continue; + branches[(*branch_count)++] = i; + return; + } + } while ((key = get_parent( key ))); +} /* create a registry key */ DECL_HANDLER(create_key) @@ -2303,15 +2340,56 @@ DECL_HANDLER(delete_key) } } -/* flush a registry key */ +/* return registry branches snaphot data for flushing key */ DECL_HANDLER(flush_key) { struct key *key = get_hkey_obj( req->hkey, 0 ); - if (key) + int branches[3], branch_count = 0, i, path_len; + char *data; + + if (!key) return; + + reply->total = 0; + reply->branch_count = 0; + if ((key->flags & KEY_DIRTY) && !(key->flags & KEY_VOLATILE)) + find_branches_for_key( key, branches, &branch_count ); + release_object( key ); + + reply->timestamp_counter = change_timestamp_counter; + for (i = 0; i < branch_count; ++i) { - /* we don't need to do anything here with the current implementation */ - release_object( key ); + if (!(save_branch_info[branches[i]].key->flags & KEY_DIRTY)) continue; + ++reply->branch_count; + path_len = strlen( save_branch_info[branches[i]].path ) + 1; + reply->total += sizeof(int) + sizeof(int) + path_len + save_registry( save_branch_info[branches[i]].key, NULL ); + } + if (reply->total > get_reply_max_size()) + { + set_error( STATUS_BUFFER_TOO_SMALL ); + return; } + + if (!(data = set_reply_data_size( reply->total ))) return; + + for (i = 0; i < branch_count; ++i) + { + if (!(save_branch_info[branches[i]].key->flags & KEY_DIRTY)) continue; + *(int *)data = branches[i]; + data += sizeof(int); + path_len = strlen( save_branch_info[branches[i]].path ) + 1; + *(int *)data = path_len; + data += sizeof(int); + memcpy( data, save_branch_info[branches[i]].path, path_len ); + data += path_len; + data += save_registry( save_branch_info[branches[i]].key, data ); + } +} + +/* clear dirty state after successful registry branch flush */ +DECL_HANDLER(flush_key_done) +{ + if (req->branch < save_branch_count) make_clean( save_branch_info[req->branch].key, req->timestamp_counter ); + else set_error( STATUS_INVALID_PARAMETER ); } /* enumerate registry subkeys */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3124
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/mountmgr.sys/mountmgr.c | 22 ++++++++++++++++++++++ server/registry.c | 26 -------------------------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/dlls/mountmgr.sys/mountmgr.c b/dlls/mountmgr.sys/mountmgr.c index ec22a8eb9bc..0729a3d52cf 100644 --- a/dlls/mountmgr.sys/mountmgr.c +++ b/dlls/mountmgr.sys/mountmgr.c @@ -611,6 +611,27 @@ static DWORD WINAPI run_loop_thread( void *arg ) return MOUNTMGR_CALL( run_loop, ¶ms ); } +static DWORD WINAPI registry_flush_thread( void *arg ) +{ + UNICODE_STRING name = RTL_CONSTANT_STRING( L"\\Registry" ); + OBJECT_ATTRIBUTES attr; + HANDLE root; + + InitializeObjectAttributes( &attr, &name, 0, 0, NULL ); + if (NtOpenKeyEx( &root, MAXIMUM_ALLOWED, &attr, 0 )) + { + ERR( "Failed opening root registry key.\n" ); + return 0; + } + + for (;;) + { + Sleep( 30000 ); + if (NtFlushKey( root )) ERR( "Failed flushing registry.\n" ); + } + + return 0; +} /* main entry point for the mount point manager driver */ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) @@ -654,6 +675,7 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) thread = CreateThread( NULL, 0, device_op_thread, NULL, 0, NULL ); CloseHandle( CreateThread( NULL, 0, run_loop_thread, thread, 0, NULL )); + CloseHandle( CreateThread( NULL, 0, registry_flush_thread, thread, 0, NULL )); #ifdef _WIN64 /* create a symlink so that the Wine port overrides key can be edited with 32-bit reg or regedit */ diff --git a/server/registry.c b/server/registry.c index 6a668c9146c..f66773978c3 100644 --- a/server/registry.c +++ b/server/registry.c @@ -125,15 +125,12 @@ static abstime_t change_timestamp_counter; static struct key *root_key; static const timeout_t ticks_1601_to_1970 = (timeout_t)86400 * (369 * 365 + 89) * TICKS_PER_SEC; -static const timeout_t save_period = 30 * -TICKS_PER_SEC; /* delay between periodic saves */ -static struct timeout_user *save_timeout_user; /* saving timer */ static enum prefix_type prefix_type; static const WCHAR wow6432node[] = {'W','o','w','6','4','3','2','N','o','d','e'}; static const WCHAR symlink_value[] = {'S','y','m','b','o','l','i','c','L','i','n','k','V','a','l','u','e'}; static const struct unicode_str symlink_str = { symlink_value, sizeof(symlink_value) }; -static void set_periodic_save_timer(void); static struct key_value *find_value( const struct key *key, const struct unicode_str *name, int *index ); /* information about where to save a registry branch */ @@ -1982,9 +1979,6 @@ void init_registry(void) release_object( hklm ); release_object( hkcu ); - /* start the periodic save timer */ - set_periodic_save_timer(); - /* create windows directories */ if (!mkdir( "drive_c/windows", 0777 )) @@ -2202,26 +2196,6 @@ done: return ret; } -/* periodic saving of the registry */ -static void periodic_save( void *arg ) -{ - int i; - - if (fchdir( config_dir_fd ) == -1) return; - save_timeout_user = NULL; - for (i = 0; i < save_branch_count; i++) - save_branch( save_branch_info[i].key, save_branch_info[i].path ); - if (fchdir( server_dir_fd ) == -1) fatal_error( "chdir to server dir: %s\n", strerror( errno )); - set_periodic_save_timer(); -} - -/* start the periodic save timer */ -static void set_periodic_save_timer(void) -{ - if (save_timeout_user) remove_timeout_user( save_timeout_user ); - save_timeout_user = add_timeout_user( save_period, periodic_save, NULL ); -} - /* save the modified registry branches to disk */ void flush_registry(void) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3124
What's the state on this? Would be useful to get this merged before trying to target https://bugs.winehq.org/show_bug.cgi?id=55026 Unless there's objections to this approach, of course. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3124#note_52016
I hate to ask again, but is there any progress on this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3124#note_84034
Is it conceptually undesirable to move the registry flush out of wineserver? In lieu of that, what can be done instead? Delegate it to a separate thread? Alter the format so that the entire file doesn't need to be written at a time? Nothing at all? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3124#note_95925
Sending the whole registry down the pipe doesn't seem like an improvement. A possibility would be to fork() and save in a child process, but ultimately a better file format is probably the way to go. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3124#note_95934
participants (5)
-
Alexandre Julliard (＠julliard) -
Elizabeth Figura (＠zfigura)
-
Fabian Maurer (＠DarkShadow44)
-
Paul Gofman
-
Paul Gofman (＠gofman)