April 2024 - Wine-Devel - list.winehq.org

[PATCH v4 10/27] ntsync: Introduce NTSYNC_IOC_SEM_READ.

by Elizabeth Figura

This corresponds to the NT syscall NtQuerySemaphore(). This returns the current count and maximum count of the semaphore. Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> --- drivers/misc/ntsync.c | 21 +++++++++++++++++++++ include/uapi/linux/ntsync.h | 1 + 2 files changed, 22 insertions(+) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index adba4657bf26..961e8d241602 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -532,6 +532,25 @@ static int ntsync_event_reset(struct ntsync_obj *event, void __user *argp) return 0; } +static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp) +{ + struct ntsync_sem_args __user *user_args = argp; + struct ntsync_sem_args args; + + if (sem->type != NTSYNC_TYPE_SEM) + return -EINVAL; + + args.sem = 0; + spin_lock(&sem->lock); + args.count = sem->u.sem.count; + args.max = sem->u.sem.max; + spin_unlock(&sem->lock); + + if (copy_to_user(user_args, &args, sizeof(args))) + return -EFAULT; + return 0; +} + static int ntsync_obj_release(struct inode *inode, struct file *file) { struct ntsync_obj *obj = file->private_data; @@ -551,6 +570,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd, switch (cmd) { case NTSYNC_IOC_SEM_POST: return ntsync_sem_post(obj, argp); + case NTSYNC_IOC_SEM_READ: + return ntsync_sem_read(obj, argp); case NTSYNC_IOC_MUTEX_UNLOCK: return ntsync_mutex_unlock(obj, argp); case NTSYNC_IOC_MUTEX_KILL: diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h index 57721f5d31ba..e298400bf25a 100644 --- a/include/uapi/linux/ntsync.h +++ b/include/uapi/linux/ntsync.h @@ -55,5 +55,6 @@ struct ntsync_wait_args { #define NTSYNC_IOC_EVENT_SET _IOR ('N', 0x88, __u32) #define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32) #define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32) +#define NTSYNC_IOC_SEM_READ _IOR ('N', 0x8b, struct ntsync_sem_args) #endif -- 2.43.0

April 17, 2024

[PATCH v4 11/27] ntsync: Introduce NTSYNC_IOC_MUTEX_READ.

by Elizabeth Figura

This corresponds to the NT syscall NtQueryMutant(). This returns the recursion count, owner, and abandoned state of the mutex. Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> --- drivers/misc/ntsync.c | 23 +++++++++++++++++++++++ include/uapi/linux/ntsync.h | 1 + 2 files changed, 24 insertions(+) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index 961e8d241602..bd043dccc9fa 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -551,6 +551,27 @@ static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp) return 0; } +static int ntsync_mutex_read(struct ntsync_obj *mutex, void __user *argp) +{ + struct ntsync_mutex_args __user *user_args = argp; + struct ntsync_mutex_args args; + int ret; + + if (mutex->type != NTSYNC_TYPE_MUTEX) + return -EINVAL; + + args.mutex = 0; + spin_lock(&mutex->lock); + args.count = mutex->u.mutex.count; + args.owner = mutex->u.mutex.owner; + ret = mutex->u.mutex.ownerdead ? -EOWNERDEAD : 0; + spin_unlock(&mutex->lock); + + if (copy_to_user(user_args, &args, sizeof(args))) + return -EFAULT; + return ret; +} + static int ntsync_obj_release(struct inode *inode, struct file *file) { struct ntsync_obj *obj = file->private_data; @@ -576,6 +597,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd, return ntsync_mutex_unlock(obj, argp); case NTSYNC_IOC_MUTEX_KILL: return ntsync_mutex_kill(obj, argp); + case NTSYNC_IOC_MUTEX_READ: + return ntsync_mutex_read(obj, argp); case NTSYNC_IOC_EVENT_SET: return ntsync_event_set(obj, argp, false); case NTSYNC_IOC_EVENT_RESET: diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h index e298400bf25a..797e8df10a3a 100644 --- a/include/uapi/linux/ntsync.h +++ b/include/uapi/linux/ntsync.h @@ -56,5 +56,6 @@ struct ntsync_wait_args { #define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32) #define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32) #define NTSYNC_IOC_SEM_READ _IOR ('N', 0x8b, struct ntsync_sem_args) +#define NTSYNC_IOC_MUTEX_READ _IOR ('N', 0x8c, struct ntsync_mutex_args) #endif -- 2.43.0

April 17, 2024

[PATCH v4 06/27] ntsync: Introduce NTSYNC_IOC_CREATE_EVENT.

by Elizabeth Figura

This correspond to the NT syscall NtCreateEvent(). An NT event holds a single bit of state denoting whether it is signaled or unsignaled. There are two types of events: manual-reset and automatic-reset. When an automatic-reset event is acquired via a wait function, its state is reset to unsignaled. Manual-reset events are not affected by wait functions. Whether the event is manual-reset, and its initial state, are specified at creation time. Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> --- drivers/misc/ntsync.c | 61 +++++++++++++++++++++++++++++++++++++ include/uapi/linux/ntsync.h | 7 +++++ 2 files changed, 68 insertions(+) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index 1e68f96bc2a6..3e125c805c00 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -25,6 +25,7 @@ enum ntsync_type { NTSYNC_TYPE_SEM, NTSYNC_TYPE_MUTEX, + NTSYNC_TYPE_EVENT, }; /* @@ -59,6 +60,10 @@ struct ntsync_obj { __u32 owner; bool ownerdead; } mutex; + struct { + bool manual; + bool signaled; + } event; } u; /* @@ -143,6 +148,8 @@ static bool is_signaled(struct ntsync_obj *obj, __u32 owner) if (obj->u.mutex.owner && obj->u.mutex.owner != owner) return false; return obj->u.mutex.count < UINT_MAX; + case NTSYNC_TYPE_EVENT: + return obj->u.event.signaled; } WARN(1, "bad object type %#x\n", obj->type); @@ -193,6 +200,10 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q, obj->u.mutex.count++; obj->u.mutex.owner = q->owner; break; + case NTSYNC_TYPE_EVENT: + if (!obj->u.event.manual) + obj->u.event.signaled = false; + break; } } wake_up_process(q->task); @@ -261,6 +272,27 @@ static void try_wake_any_mutex(struct ntsync_obj *mutex) } } +static void try_wake_any_event(struct ntsync_obj *event) +{ + struct ntsync_q_entry *entry; + + lockdep_assert_held(&event->lock); + + list_for_each_entry(entry, &event->any_waiters, node) { + struct ntsync_q *q = entry->q; + int signaled = -1; + + if (!event->u.event.signaled) + break; + + if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) { + if (!event->u.event.manual) + event->u.event.signaled = false; + wake_up_process(q->task); + } + } +} + /* * Actually change the semaphore state, returning -EOVERFLOW if it is made * invalid. @@ -569,6 +601,30 @@ static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp) return put_user(fd, &user_args->mutex); } +static int ntsync_create_event(struct ntsync_device *dev, void __user *argp) +{ + struct ntsync_event_args __user *user_args = argp; + struct ntsync_event_args args; + struct ntsync_obj *event; + int fd; + + if (copy_from_user(&args, argp, sizeof(args))) + return -EFAULT; + + event = ntsync_alloc_obj(dev, NTSYNC_TYPE_EVENT); + if (!event) + return -ENOMEM; + event->u.event.manual = args.manual; + event->u.event.signaled = args.signaled; + fd = ntsync_obj_get_fd(event); + if (fd < 0) { + kfree(event); + return fd; + } + + return put_user(fd, &user_args->event); +} + static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd) { struct file *file = fget(fd); @@ -702,6 +758,9 @@ static void try_wake_any_obj(struct ntsync_obj *obj) case NTSYNC_TYPE_MUTEX: try_wake_any_mutex(obj); break; + case NTSYNC_TYPE_EVENT: + try_wake_any_event(obj); + break; } } @@ -890,6 +949,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd, void __user *argp = (void __user *)parm; switch (cmd) { + case NTSYNC_IOC_CREATE_EVENT: + return ntsync_create_event(dev, argp); case NTSYNC_IOC_CREATE_MUTEX: return ntsync_create_mutex(dev, argp); case NTSYNC_IOC_CREATE_SEM: diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h index 1bff8f19d6d9..0d133f2eaf0b 100644 --- a/include/uapi/linux/ntsync.h +++ b/include/uapi/linux/ntsync.h @@ -22,6 +22,12 @@ struct ntsync_mutex_args { __u32 count; }; +struct ntsync_event_args { + __u32 event; + __u32 manual; + __u32 signaled; +}; + #define NTSYNC_WAIT_REALTIME 0x1 struct ntsync_wait_args { @@ -41,6 +47,7 @@ struct ntsync_wait_args { #define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args) #define NTSYNC_IOC_WAIT_ALL _IOWR('N', 0x83, struct ntsync_wait_args) #define NTSYNC_IOC_CREATE_MUTEX _IOWR('N', 0x84, struct ntsync_sem_args) +#define NTSYNC_IOC_CREATE_EVENT _IOWR('N', 0x87, struct ntsync_event_args) #define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32) #define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args) -- 2.43.0

April 17, 2024

Re: [PATCH v4 00/30] NT synchronization primitive driver

by Elizabeth Figura

On Tuesday, 16 April 2024 16:18:24 CDT Elizabeth Figura wrote: > On Tuesday, 16 April 2024 03:14:21 CDT Peter Zijlstra wrote: > > I don't support GE has it in his builds? Last time I tried, building > > Wine was a bit of a pain. > > It doesn't seem so. I tried to build a GE-compatible ntsync build, uploaded > here (thanks Arek for hosting): > > https://f002.backblazeb2.com/file/wine-ntsync/ntsync-wine.tar.xz Oops, the initial version I uploaded had broken paths. Should be fixed now. (It's also broken on an unpatched kernel unless explicitly disabled with WINE_DISABLE_FAST_SYNC=1. Not sure what I messed up there—it should fall back cleanly—but hopefully shouldn't be too important for testing.)

April 16, 2024

Re: [PATCH v4 00/30] NT synchronization primitive driver

by Elizabeth Figura

On Tuesday, 16 April 2024 03:14:21 CDT Peter Zijlstra wrote: > On Mon, Apr 15, 2024 at 08:08:10PM -0500, Elizabeth Figura wrote: > > This patch series implements a new char misc driver, /dev/ntsync, which is > > used to implement Windows NT synchronization primitives. > > This patch series does not apply to anything I have at hand. Nor does it > state anything explicit to put it on top of. It was written to apply against the 'char-misc-next' branch of gregkh/char- misc.git. I'll make a note of that next time, sorry for the inconvenience. > > Hence I would like to request review from someone familiar with locking to > > make sure that the usage of low-level kernel primitives is correct and > > that the wait queues work as intended, and to that end I've CC'd the > > locking maintainers. > I am sadly very limited atm, but I'll try and read through it. If only I > could apply... > > > == Patches == > > > > The intended semantics of the patches are broadly intended to match those > > of the corresponding Windows functions. For those not already familiar > > with the Windows functions (or their undocumented behaviour), patch 27/27 > > provides a detailed specification, and individual patches also include a > > brief description of the API they are implementing. > > > > The patches making use of this driver in Wine can be retrieved or browsed here: > > https://repo.or.cz/wine/zf.git/shortlog/refs/heads/ntsync5 > > I don't support GE has it in his builds? Last time I tried, building > Wine was a bit of a pain. It doesn't seem so. I tried to build a GE-compatible ntsync build, uploaded here (thanks Arek for hosting): https://f002.backblazeb2.com/file/wine-ntsync/ntsync-wine.tar.xz > > Some aspects of the implementation may deserve particular comment: > > > > * In the interest of performance, each object is governed only by a single > > > > spinlock. However, NTSYNC_IOC_WAIT_ALL requires that the state of > > multiple > > objects be changed as a single atomic operation. In order to achieve > > this, we first take a device-wide lock ("wait_all_lock") any time we > > are going to lock more than one object at a time. > > > > The maximum number of objects that can be used in a vectored wait, and > > therefore the maximum that can be locked simultaneously, is 64. This > > number is NT's own limit. > > > > The acquisition of multiple spinlocks will degrade performance. This is > > a > > conscious choice, however. Wait-for-all is known to be a very rare > > operation in practice, especially with counts that approach the > > maximum, and it is the intent of the ntsync driver to optimize > > wait-for-any at the expense of wait-for-all as much as possible. > > Per the example of percpu-rwsem, it would be possible to create a > mutex-spinlock hybrid scheme, where single locks are spinlocks while > held, but can block when the global thing is pending. And the global > lock is always mutex like. > > If all that is worth it, I don't know. Nesting 64 spinlocks doesn't give > me warm and fuzzy feelings though. Is the concern about poor performance when ntsync is in use, or is nesting a lot of spinlocks like that something that could cause problems for unrelated tasks? I'm not familiar enough with the scheduler to know if this can be abused. I think we don't care about performance problems within Wine, at least. FWIW, the scheme here is actually similar to what Windows does (as described by one of their kernel engineers), although slightly different. NT nests spinlocks as well, but instead of using the outer lock like our "wait_all_lock" to prevent lock inversion, they instead sort the inner locks (by address, I assume). If there's deeper problems... I can look into (ab)using a rwlock for this purpose, at least for now. In any case making wait_all_lock into a sleeping mutex instead of a spinlock should be fine. I'll rerun performance tests but I don't expect it to cause any problems.

April 16, 2024

Re: [PATCH v4 00/30] NT synchronization primitive driver

by Elizabeth Figura

On Tuesday, 16 April 2024 11:05:53 CDT Peter Zijlstra wrote: > On Mon, Apr 15, 2024 at 08:08:10PM -0500, Elizabeth Figura wrote: > > The intended semantics of the patches are broadly intended to match those > > of the corresponding Windows functions. For those not already familiar > > with the Windows functions (or their undocumented behaviour), patch 27/27 > > provides a detailed specification, and individual patches also include a > > brief description of the API they are implementing. > > You happen to have a readable copy of patch 27 around? RST is utter > garbage to read :/ An HTML copy should be available here now (thanks Arek): https://ivyl.gg/ntsync/ntsync.html Let me know if that's good enough or if I should try to render it into plain text somehow.

April 16, 2024

Re: [PATCH v4 00/30] NT synchronization primitive driver

by Elizabeth Figura

On Tuesday, 16 April 2024 11:19:17 CDT Peter Zijlstra wrote: > On Tue, Apr 16, 2024 at 05:53:45PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 16, 2024 at 05:50:14PM +0200, Peter Zijlstra wrote: > > > On Tue, Apr 16, 2024 at 10:14:21AM +0200, Peter Zijlstra wrote: > > > > > Some aspects of the implementation may deserve particular comment: > > > > > > > > > > * In the interest of performance, each object is governed only by a > > > > > single > > > > > > > > > > spinlock. However, NTSYNC_IOC_WAIT_ALL requires that the state of > > > > > multiple > > > > > objects be changed as a single atomic operation. In order to > > > > > achieve this, we first take a device-wide lock ("wait_all_lock") > > > > > any time we are going to lock more than one object at a time. > > > > > > > > > > The maximum number of objects that can be used in a vectored wait, > > > > > and > > > > > therefore the maximum that can be locked simultaneously, is 64. > > > > > This number is NT's own limit. > > > > > > AFAICT: > > > spin_lock(&dev->wait_all_lock); > > > > > > list_for_each_entry(entry, &obj->all_waiters, node) > > > > > > for (i=0; i<count; i++) > > > > > > spin_lock_nest_lock(q->entries[i].obj->lock, > > > &dev->wait_all_lock); > > > > > > Where @count <= NTSYNC_MAX_WAIT_COUNT. > > > > > > So while this nests at most 65 spinlocks, there is no actual bound on > > > the amount of nested lock sections in total. That is, all_waiters list > > > can be grown without limits. > > > > > > Can we pretty please make wait_all_lock a mutex ? That should be fine, at least. > > Hurmph, it's worse, you do that list walk while holding some obj->lock > > spinlokc too. Still need to figure out how all that works.... > > So the point of having that other lock around is so that things like: > > try_wake_all_obj(dev, sem) > try_wake_any_sem(sem) > > are done under the same lock? The point of having the other lock around is that try_wake_all() needs to lock multiple objects at the same time. It's a way of avoiding lock inversion. Consider task A does a wait-for-all on objects X, Y, Z. Then task B signals Y, so we do try_wake_all_obj() on Y, which does try_wake_all() on A's queue entry; that needs to check X and Z and consume the state of all three objects atomically. Another task could be trying to signal Z at the same time and could hit a task waiting on Z, Y, X, and that causes inversion. The simple and easy way to implement everything is just to have a global lock on the whole device, but this is kind of known to be a performance bottleneck (this was NT's BKL, and they ditched it starting with Vista or 7 or something). Instead we use a lock per object, and normally in the wait-for-any case we only ever need to grab one lock at a time, but when we need to do a wait-for- all we need to lock multiple objects at once, and we grab the outer lock to avoid potential lock inversion. > Where I seem to note that both those functions do that same list > iteration. Over different lists. I don't know if there's a better way to name things to make that clearer. There's the "any" wait queue, which tasks which do a wait-for-any add themselves to, and the "all" wait queue, which tasks that do a wait-for-all add themselves to. Signaling an object could potentially wake up either one, but checking whether a task is eligible is a different process.

April 16, 2024

Re: Bumping the minimum supported macOS version

by Dean Greer

Sorry no longer supports below macOS Catalina. On Mon, Apr 15, 2024 at 10:18 PM Dean Greer <gcenx83(a)gmail.com> wrote: > > Dean, do you know if there are many users of your ports on 10.13/10.14? > > The main Ports no longer support macOS Sonoma (wine-stable, wine-devel & > wine-staging), I do provide alternative Portfiles that work for all legacy > versions of macOS from 10.6.8 to 10.14. > > It’s also a nightmare buiding on 10.14 thanks to some macports maintainers > not believing that Xcode10 Command Line Tools provided a headers package > that enabled the ability to build for i386 & x86_64. > > > Your MacPorts packages still support those, right? > > If your referring to the Winehq packages installable via brew cask > system?, no those only support macOS Catalina or later, there’s really been > no complaints. > > Something else to keep in mind is these legacy versions of macOS also have > expired certs. > > On Mon, Apr 15, 2024 at 7:15 PM Brendan Shanks <bshanks(a)codeweavers.com> > wrote: > >> >> > On Apr 11, 2024, at 11:14 AM, Tim Clem <tclem(a)codeweavers.com> wrote: >> > >> > I would prefer 10.15 in particular so we could drop support for native >> (i.e. macho) 32-bit binaries. That would allow us to use modern Obj-C >> runtime features (e.g. auto-synthesis) and eventually consider ARC. It's >> also worth noting that winehq's gitlab CI runs 10.15. >> >> I’m totally on board with raising the minimum version to 10.13. >> I’m slightly more hesitant to go to 10.15, just since there are a lot of >> Macs and software that can only run (on) 10.13 or 10.14. But Firefox, >> Chrome, and Steam have all raised their minimum versions to 10.15 in the >> last year (and they were among the last to do so). We can’t support them >> forever, and anyone who needs to run 10.13 or 10.14 can keep running Wine >> 9.0. Ultimately I think I’m good with 10.15, although I’m curious whether >> Dean has any more info. >> >> Dean, do you know if there are many users of your ports on 10.13/10.14? >> Your MacPorts packages still support those, right? >> >> (Also for those who aren’t aware, CrossOver requires 10.15 starting with >> this year’s version 24). >> >> Brendan > >

April 16, 2024

Re: Bumping the minimum supported macOS version

by Dean Greer

> Dean, do you know if there are many users of your ports on 10.13/10.14? The main Ports no longer support macOS Sonoma (wine-stable, wine-devel & wine-staging), I do provide alternative Portfiles that work for all legacy versions of macOS from 10.6.8 to 10.14. It’s also a nightmare buiding on 10.14 thanks to some macports maintainers not believing that Xcode10 Command Line Tools provided a headers package that enabled the ability to build for i386 & x86_64. > Your MacPorts packages still support those, right? If your referring to the Winehq packages installable via brew cask system?, no those only support macOS Catalina or later, there’s really been no complaints. Something else to keep in mind is these legacy versions of macOS also have expired certs. On Mon, Apr 15, 2024 at 7:15 PM Brendan Shanks <bshanks(a)codeweavers.com> wrote: > > > On Apr 11, 2024, at 11:14 AM, Tim Clem <tclem(a)codeweavers.com> wrote: > > > > I would prefer 10.15 in particular so we could drop support for native > (i.e. macho) 32-bit binaries. That would allow us to use modern Obj-C > runtime features (e.g. auto-synthesis) and eventually consider ARC. It's > also worth noting that winehq's gitlab CI runs 10.15. > > I’m totally on board with raising the minimum version to 10.13. > I’m slightly more hesitant to go to 10.15, just since there are a lot of > Macs and software that can only run (on) 10.13 or 10.14. But Firefox, > Chrome, and Steam have all raised their minimum versions to 10.15 in the > last year (and they were among the last to do so). We can’t support them > forever, and anyone who needs to run 10.13 or 10.14 can keep running Wine > 9.0. Ultimately I think I’m good with 10.15, although I’m curious whether > Dean has any more info. > > Dean, do you know if there are many users of your ports on 10.13/10.14? > Your MacPorts packages still support those, right? > > (Also for those who aren’t aware, CrossOver requires 10.15 starting with > this year’s version 24). > > Brendan

April 16, 2024

Re: Bumping the minimum supported macOS version

by Brendan Shanks

> On Apr 11, 2024, at 11:14 AM, Tim Clem <tclem(a)codeweavers.com> wrote: > > I would prefer 10.15 in particular so we could drop support for native (i.e. macho) 32-bit binaries. That would allow us to use modern Obj-C runtime features (e.g. auto-synthesis) and eventually consider ARC. It's also worth noting that winehq's gitlab CI runs 10.15. I’m totally on board with raising the minimum version to 10.13. I’m slightly more hesitant to go to 10.15, just since there are a lot of Macs and software that can only run (on) 10.13 or 10.14. But Firefox, Chrome, and Steam have all raised their minimum versions to 10.15 in the last year (and they were among the last to do so). We can’t support them forever, and anyone who needs to run 10.13 or 10.14 can keep running Wine 9.0. Ultimately I think I’m good with 10.15, although I’m curious whether Dean has any more info. Dean, do you know if there are many users of your ports on 10.13/10.14? Your MacPorts packages still support those, right? (Also for those who aren’t aware, CrossOver requires 10.15 starting with this year’s version 24). Brendan

April 15, 2024