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

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

by Elizabeth Figura

On Wednesday, April 17, 2024 3:02:13 PM CDT Elizabeth Figura wrote: > > > Except for the "unowned" semantics of zero, the actual value of the > > > owner identifier is not interpreted by the ntsync driver at all. The > > > intended use is to store a thread identifier; however, the ntsync > > > driver does not actually validate that a calling thread provides > > > consistent or unique identifiers. > > > > Why not verify it? Seems simple enough to put in a TID check, esp. if NT > > mandates the same. > > I mostly figured it'd be simplest to leave the driver completely > agnostic, but I don't think there's any reason we can't use the real > TID for most calls. While trying to implement this I did realize a reason: if a Linux thread dies and a new Wine thread is created which happens to have the same Linux TID *before* Wine notices the thread death, that thread's TID will be conflated with the thread that died. I don't think we can guarantee that we notice thread death before we notice a request to create a new Wine thread. Using Wine-managed TIDs avoids this by virtue of ensuring that a Wine TID is not reused until the associated Wine thread has been cleaned up.

May 15, 2024

Re: Proposal: changing the default bug status (from 'UNCONFIRMED' to 'NEW')

by Jinoh Kang

On Fri, May 3, 2024 at 8:30 AM Gijs Vermeulen <gijsvrm(a)gmail.com> wrote: > > Hi! > > I do agree that the 'UNCONFIRMED' & 'NEW' statuses lead to confusion. I opened bug 50815[1] in 2021 for this reason. > No agreement was reached, however. > > Having some sort of 'TRIAGED' status or something similar, does seem like a good idea. At least to have some sort of status that reflects > that the bug has been looked at/triaged. Maybe we can then: 1. Rename 'NEW' -> 'TRIANGED', and then 2. Rename 'UNCONFIRMED' -> 'NEW'. -- Sincerely, Jinoh Kang

May 15, 2024

Re: [PATCH v4 02/27] ntsync: Introduce NTSYNC_IOC_WAIT_ALL.

by Elizabeth Figura

On Friday, April 19, 2024 11:28:14 AM CDT Peter Zijlstra wrote: > On Thu, Apr 18, 2024 at 11:35:11AM +0200, Peter Zijlstra wrote: > > On Wed, Apr 17, 2024 at 03:03:05PM -0500, Elizabeth Figura wrote: > > > Ach. I wrote this with the idea that the race isn't meaningful, but > > > looking at it again you're right—there is a harmful race here. > > > > > > I think it should be fixable by moving the atomic_read inside the lock, > > > though. > > > > Right, I've ended up with the (as yet untested) below. I'll see if I can > > find time later to actually test things. > > Latest hackery... I tried testing this but I'm not having luck using the > patched wine as per the other email. > I converted the rest of the direct uses of spin_lock() using the below patch and tested it myself, and it passes Wine tests. As far as I can tell the logic is correct, too; I couldn't find any races. I'll incorporate these changes into the next revision, unless there's a good reason not to. --- --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -569,17 +569,19 @@ static int ntsync_event_set(struct ntsync_obj *event, void __user *argp, bool pu static int ntsync_event_reset(struct ntsync_obj *event, void __user *argp) { + struct ntsync_device *dev = event->dev; __u32 prev_state; + bool all; if (event->type != NTSYNC_TYPE_EVENT) return -EINVAL; - spin_lock(&event->lock); + all = ntsync_lock_obj(dev, event); prev_state = event->u.event.signaled; event->u.event.signaled = false; - spin_unlock(&event->lock); + ntsync_unlock_obj(dev, event, all); if (put_user(prev_state, (__u32 __user *)argp)) return -EFAULT; @@ -590,16 +592,21 @@ static int ntsync_event_reset(struct ntsync_obj *event, void __user *argp) static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp) { struct ntsync_sem_args __user *user_args = argp; + struct ntsync_device *dev = sem->dev; struct ntsync_sem_args args; + bool all; if (sem->type != NTSYNC_TYPE_SEM) return -EINVAL; args.sem = 0; - spin_lock(&sem->lock); + + all = ntsync_lock_obj(dev, sem); + args.count = sem->u.sem.count; args.max = sem->u.sem.max; - spin_unlock(&sem->lock); + + ntsync_unlock_obj(dev, sem, all); if (copy_to_user(user_args, &args, sizeof(args))) return -EFAULT; @@ -609,18 +616,23 @@ static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp) static int ntsync_mutex_read(struct ntsync_obj *mutex, void __user *argp) { struct ntsync_mutex_args __user *user_args = argp; + struct ntsync_device *dev = mutex->dev; struct ntsync_mutex_args args; + bool all; int ret; if (mutex->type != NTSYNC_TYPE_MUTEX) return -EINVAL; args.mutex = 0; - spin_lock(&mutex->lock); + + all = ntsync_lock_obj(dev, mutex); + args.count = mutex->u.mutex.count; args.owner = mutex->u.mutex.owner; ret = mutex->u.mutex.ownerdead ? -EOWNERDEAD : 0; - spin_unlock(&mutex->lock); + + ntsync_unlock_obj(dev, mutex, all); if (copy_to_user(user_args, &args, sizeof(args))) return -EFAULT; @@ -630,16 +642,21 @@ static int ntsync_mutex_read(struct ntsync_obj *mutex, void __user *argp) static int ntsync_event_read(struct ntsync_obj *event, void __user *argp) { struct ntsync_event_args __user *user_args = argp; + struct ntsync_device *dev = event->dev; struct ntsync_event_args args; + bool all; if (event->type != NTSYNC_TYPE_EVENT) return -EINVAL; args.event = 0; - spin_lock(&event->lock); + + all = ntsync_lock_obj(dev, event); + args.manual = event->u.event.manual; args.signaled = event->u.event.signaled; - spin_unlock(&event->lock); + + ntsync_unlock_obj(dev, event, all); if (copy_to_user(user_args, &args, sizeof(args))) return -EFAULT; @@ -962,6 +979,7 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp) __u32 i, total_count; struct ntsync_q *q; int signaled; + bool all; int ret; if (copy_from_user(&args, argp, sizeof(args))) @@ -981,9 +999,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp) struct ntsync_q_entry *entry = &q->entries[i]; struct ntsync_obj *obj = entry->obj; - spin_lock(&obj->lock); + all = ntsync_lock_obj(dev, obj); list_add_tail(&entry->node, &obj->any_waiters); - spin_unlock(&obj->lock); + ntsync_unlock_obj(dev, obj, all); } /* @@ -1000,9 +1018,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp) if (atomic_read(&q->signaled) != -1) break; - spin_lock(&obj->lock); + all = ntsync_lock_obj(dev, obj); try_wake_any_obj(obj); - spin_unlock(&obj->lock); + ntsync_unlock_obj(dev, obj, all); } /* sleep */ @@ -1015,9 +1033,9 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp) struct ntsync_q_entry *entry = &q->entries[i]; struct ntsync_obj *obj = entry->obj; - spin_lock(&obj->lock); + all = ntsync_lock_obj(dev, obj); list_del(&entry->node); - spin_unlock(&obj->lock); + ntsync_unlock_obj(dev, obj, all); put_obj(obj); } @@ -1075,9 +1093,9 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp) struct ntsync_q_entry *entry = &q->entries[args.count]; struct ntsync_obj *obj = entry->obj; - spin_lock_nest_lock(&obj->lock, &dev->wait_all_lock); + dev_lock_obj(dev, obj); list_add_tail(&entry->node, &obj->any_waiters); - spin_unlock(&obj->lock); + dev_unlock_obj(dev, obj); } /* check if we are already signaled */ @@ -1095,9 +1113,9 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp) struct ntsync_obj *obj = q->entries[args.count].obj; if (atomic_read(&q->signaled) == -1) { - spin_lock(&obj->lock); + dev_lock_obj(dev, obj); try_wake_any_obj(obj); - spin_unlock(&obj->lock); + dev_unlock_obj(dev, obj); } } @@ -1127,9 +1145,9 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp) struct ntsync_q_entry *entry = &q->entries[args.count]; struct ntsync_obj *obj = entry->obj; - spin_lock_nest_lock(&obj->lock, &dev->wait_all_lock); + dev_lock_obj(dev, obj); list_del(&entry->node); - spin_unlock(&obj->lock); + dev_unlock_obj(dev, obj); put_obj(obj); }

May 14, 2024

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

by Elizabeth Figura

On Friday, April 19, 2024 3:46:07 PM CDT Elizabeth Figura wrote: > On Friday, 19 April 2024 11:16:11 CDT Peter Zijlstra wrote: > > > On Tue, Apr 16, 2024 at 05:18:56PM -0500, Elizabeth Figura wrote: > > > > > 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.) > > > > > > So I've tried using that wine build with lutris, and I can't get it to > > start EGS or anything else. > > > > I even added a printk to the ntsync driver for every open, to see if it > > gets that far, but I'm not even getting that :/ > > > That's odd, it works for me, both as a standalone build and with > lutris... > > Does /dev/ntsync exist (module is loaded) and have nonzero permissions? > I forgot to mention that's necessary, sorry. > > Otherwise I can try to look at an strace, or a Wine debug log. I don't > think there's an easy way to get the latter with Lutris, but something > like `WINEDEBUG=+all ./wine winecfg 2>log` should work. > > It's also possible that the build was made against too new libraries. I've created a new build, built and tested on stock Debian 12, and with some extra debugging: https://f002.backblazeb2.com/file/wine-ntsync/ntsync-wine2.tar.xz Hopefully that's good enough to test with, or at least gives more of a hint as to why it fails?

May 7, 2024

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

by Elizabeth Figura

On Friday, April 19, 2024 3:46:07 PM CDT Elizabeth Figura wrote: > On Friday, 19 April 2024 11:16:11 CDT Peter Zijlstra wrote: > > > On Tue, Apr 16, 2024 at 05:18:56PM -0500, Elizabeth Figura wrote: > > > > > 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.) > > > > > > So I've tried using that wine build with lutris, and I can't get it to > > start EGS or anything else. > > > > I even added a printk to the ntsync driver for every open, to see if it > > gets that far, but I'm not even getting that :/ > > > That's odd, it works for me, both as a standalone build and with > lutris... > > Does /dev/ntsync exist (module is loaded) and have nonzero permissions? > I forgot to mention that's necessary, sorry. > > Otherwise I can try to look at an strace, or a Wine debug log. I don't > think there's an easy way to get the latter with Lutris, but something > like `WINEDEBUG=+all ./wine winecfg 2>log` should work. It's also possible I made that build against a too-new distribution. I've created a new build against Debian 12, which is hopefully better, and it has some extra debug logging: https://f002.backblazeb2.com/file/wine-ntsync/ntsync-wine2.tar.xz Hopefully that's functional enough to test with, or at least give more a hint as to why it doesn't work?

May 7, 2024

Wine staging 9.8 release

by Alistair Leslie-Hughes

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary since last release * Rebased to current wine 9.8 (430 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * widl: Make automatic dispid generation scheme better match what midl does. * widl: Create library block index right after the CompObj one. * widl: Set the lowest bit in the param name to indicate whether type description follows the name. * widl: Add support for function parameter flags to SLTG typelib generator. * widl: Avoid relying on side effects when marking function index as the last one. * stdole32.tlb: Compile typelib with --oldtlb. Removed (No longer required). * ntdll-CriticalSection * Pipelight * winex11-XEMBED Added: * [46012] d3d9: Implement ID3DXEffect::SetRawValue. Updated: * vkd3d-latest * odbc-remove-unixodbc * ddraw-GetPickRecords Where can you help * Run Steam/Battle.net/GOG/UPlay/Epic * Test your favorite game. * Test your favorite applications. * Improve staging patches and get them accepted upstream. * Suggest patches to be included in staging. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

May 4, 2024

Re: Proposal: changing the default bug status (from 'UNCONFIRMED' to 'NEW')

by Gijs Vermeulen

Hi! I do agree that the 'UNCONFIRMED' & 'NEW' statuses lead to confusion. I opened bug 50815[1] in 2021 for this reason. No agreement was reached, however. Having some sort of 'TRIAGED' status or something similar, does seem like a good idea. At least to have some sort of status that reflects that the bug has been looked at/triaged. Kind regards, Gijs [1] https://bugs.winehq.org/show_bug.cgi?id=50815 Op vr 3 mei 2024 om 01:19 schreef Austin English <austinenglish(a)gmail.com>: > Howdy all, > > Currently, unprivileged users have their bugs set to 'UNCONFIRMED' by > default (while privileged users have the option of > 'UNCONFIRMED'/'NEW'/'ASSIGNED'). > > As a general rule, no one seems to care if a bug is confirmed ('NEW') > or not. This often leads to confusion for users (for example recently, > [1]), who think that because their bug is 'UNCONFIRMED', it won't get > attention. > > Because this distinction is useless, I'd like to propose that we make > a few changes to Bugzilla: > * change the default status for newly submitted bugs to 'NEW' (here: > https://bugs.winehq.org/editworkflow.cgi) > * do a bulk status update of existing bugs from 'UNCONFIRMED' to 'NEW' > * disable the 'UNCONFIRMED' status (here: > https://bugs.winehq.org/editproducts.cgi?action=edit&product=Wine) > > Thoughts/concerns? > > [1] https://bugs.winehq.org/show_bug.cgi?id=50876 > > -- > -Austin > GPG: 267B CC1F 053F 0749 (expires 2026-03-03) > > >

May 2, 2024

Proposal: changing the default bug status (from 'UNCONFIRMED' to 'NEW')

by Austin English

Howdy all, Currently, unprivileged users have their bugs set to 'UNCONFIRMED' by default (while privileged users have the option of 'UNCONFIRMED'/'NEW'/'ASSIGNED'). As a general rule, no one seems to care if a bug is confirmed ('NEW') or not. This often leads to confusion for users (for example recently, [1]), who think that because their bug is 'UNCONFIRMED', it won't get attention. Because this distinction is useless, I'd like to propose that we make a few changes to Bugzilla: * change the default status for newly submitted bugs to 'NEW' (here: https://bugs.winehq.org/editworkflow.cgi) * do a bulk status update of existing bugs from 'UNCONFIRMED' to 'NEW' * disable the 'UNCONFIRMED' status (here: https://bugs.winehq.org/editproducts.cgi?action=edit&product=Wine) Thoughts/concerns? [1] https://bugs.winehq.org/show_bug.cgi?id=50876 -- -Austin GPG: 267B CC1F 053F 0749 (expires 2026-03-03)

May 2, 2024