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

Re: [PATCH v3 04/30] ntsync: Introduce NTSYNC_IOC_WAIT_ANY.

by Elizabeth Figura

On Thursday, 11 April 2024 08:34:23 CDT Greg Kroah-Hartman wrote: > On Thu, Mar 28, 2024 at 07:05:55PM -0500, Elizabeth Figura wrote: > > This corresponds to part of the functionality of the NT syscall > > NtWaitForMultipleObjects(). Specifically, it implements the behaviour where > > the third argument (wait_any) is TRUE, and it does not handle alertable waits. > > Those features have been split out into separate patches to ease review. > > > > NTSYNC_IOC_WAIT_ANY is a vectored wait function similar to poll(). Unlike > > poll(), it "consumes" objects when they are signaled. For semaphores, this means > > decreasing one from the internal counter. At most one object can be consumed by > > this function. > > > > Up to 64 objects can be waited on at once. As soon as one is signaled, the > > object with the lowest index is consumed, and that index is returned via the > > "index" field. > > So it's kind of like our internal locks already? Or futex? Striking the right balance of explaining the problem space without inundating the reader with information has been tricky; I'll do my best to try to explain here. The primitives include mutexes and semaphores, like our internal locks. I don't really want to compare them to futexes because futexes don't have internal state. However NT's primitives are *way* more complicated. The big part of it is they consume state in a way that usual wait functions don't, and as if that weren't enough, you can do operations with them like wait-for-all (wait for all objects to be simultaneously signaled and atomically consume them) or pulse (signal an object without changing its state). None of this can be expressed with poll or futex. You can't even express those operations with wait_event() etc. We really need to replace the entire wait queue and use schedule() + wake_up_process() directly. ntsync_q is the wait queue struct in this. They're also really ugly things to do; they only exist for compatibility reasons, and retrofitting support into anything would complicate and slow down hot paths. > > A timeout is supported. The timeout is passed as a u64 nanosecond value, which > > represents absolute time measured against either the MONOTONIC or REALTIME clock > > (controlled by the flags argument). If U64_MAX is passed, the ioctl waits > > indefinitely. > > > > This ioctl validates that all objects belong to the relevant device. This is not > > necessary for any technical reason related to NTSYNC_IOC_WAIT_ANY, but will be > > necessary for NTSYNC_IOC_WAIT_ALL introduced in the following patch. > > > > Two u32s of padding are left in the ntsync_wait_args structure; one will be used > > by a patch later in the series (which is split out to ease review). > > > > Signed-off-by: Elizabeth Figura <zfigura(a)codeweavers.com> > > --- > > drivers/misc/ntsync.c | 250 ++++++++++++++++++++++++++++++++++++ > > include/uapi/linux/ntsync.h | 16 +++ > > 2 files changed, 266 insertions(+) > > > > diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c > > index 3c2f743c58b0..c6f84a5fc8c0 100644 > > --- a/drivers/misc/ntsync.c > > +++ b/drivers/misc/ntsync.c > > @@ -6,11 +6,16 @@ > > */ > > > > #include <linux/anon_inodes.h> > > +#include <linux/atomic.h> > > #include <linux/file.h> > > #include <linux/fs.h> > > +#include <linux/hrtimer.h> > > +#include <linux/ktime.h> > > #include <linux/miscdevice.h> > > #include <linux/module.h> > > #include <linux/overflow.h> > > +#include <linux/sched.h> > > +#include <linux/sched/signal.h> > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > #include <uapi/linux/ntsync.h> > > @@ -30,6 +35,8 @@ enum ntsync_type { > > * > > * Both rely on struct file for reference counting. Individual > > * ntsync_obj objects take a reference to the device when created. > > + * Wait operations take a reference to each object being waited on for > > + * the duration of the wait. > > */ > > > > struct ntsync_obj { > > @@ -47,12 +54,56 @@ struct ntsync_obj { > > __u32 max; > > } sem; > > } u; > > + > > + struct list_head any_waiters; > > +}; > > + > > +struct ntsync_q_entry { > > + struct list_head node; > > + struct ntsync_q *q; > > + struct ntsync_obj *obj; > > + __u32 index; > > +}; > > + > > +struct ntsync_q { > > + struct task_struct *task; > > + __u32 owner; > > + > > + /* > > + * Protected via atomic_try_cmpxchg(). Only the thread that wins the > > + * compare-and-swap may actually change object states and wake this > > + * task. > > + */ > > + atomic_t signaled; > > This feels odd, why are you duplicating a normal lock functionality > here? ntsync_q represents a single waiter (like struct wait_queue_entry). In short, waiting is a destructive operation; it changes the state of the primitives waited on. If a waiter is woken successfully then it must have consumed the state of exactly one object. Therefore, if task A is waiting on two primitives X and Y, and those primitives are respectively woken at the same time by tasks B and C, we need a way to ensure that B and C don't both wake A and consume the state of X and Y. Only one of them should win. We could do that with a lock on the ntsync_q struct, but having a single variable with atomic-test-and-set achieves the same thing while being lock-free. > > + > > + __u32 count; > > + struct ntsync_q_entry entries[]; > > }; > > > > struct ntsync_device { > > struct file *file; > > }; > > > > +static void try_wake_any_sem(struct ntsync_obj *sem) > > +{ > > + struct ntsync_q_entry *entry; > > + > > + lockdep_assert_held(&sem->lock); > > + > > + list_for_each_entry(entry, &sem->any_waiters, node) { > > + struct ntsync_q *q = entry->q; > > + int signaled = -1; > > + > > + if (!sem->u.sem.count) > > + break; > > + > > + if (atomic_try_cmpxchg(&q->signaled, &signaled, entry->index)) { > > + sem->u.sem.count--; > > + wake_up_process(q->task); > > + } > > You are waking up _all_ "locks" that with the atomic_try_cmpxchg() call, > right? Not just the "first". > > Or am I confused? This is looping over all tasks trying to lock / acquire "sem", and waking them (assuming something else didn't wake them first) while decrementing "sem" state accordingly. > > + } > > +} > > + > > /* > > * Actually change the semaphore state, returning -EOVERFLOW if it is made > > * invalid. > > @@ -88,6 +139,8 @@ static int ntsync_sem_post(struct ntsync_obj *sem, void __user *argp) > > > > prev_count = sem->u.sem.count; > > ret = post_sem_state(sem, args); > > + if (!ret) > > + try_wake_any_sem(sem); > > > > spin_unlock(&sem->lock); > > > > @@ -141,6 +194,7 @@ static struct ntsync_obj *ntsync_alloc_obj(struct ntsync_device *dev, > > obj->dev = dev; > > get_file(dev->file); > > spin_lock_init(&obj->lock); > > + INIT_LIST_HEAD(&obj->any_waiters); > > > > return obj; > > } > > @@ -191,6 +245,200 @@ static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp) > > return put_user(fd, &user_args->sem); > > } > > > > +static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd) > > +{ > > + struct file *file = fget(fd); > > + struct ntsync_obj *obj; > > + > > + if (!file) > > + return NULL; > > + > > + if (file->f_op != &ntsync_obj_fops) { > > + fput(file); > > + return NULL; > > + } > > + > > + obj = file->private_data; > > + if (obj->dev != dev) { > > + fput(file); > > + return NULL; > > + } > > + > > + return obj; > > +} > > + > > +static void put_obj(struct ntsync_obj *obj) > > +{ > > + fput(obj->file); > > +} > > + > > +static int ntsync_schedule(const struct ntsync_q *q, const struct ntsync_wait_args *args) > > +{ > > + ktime_t timeout = ns_to_ktime(args->timeout); > > + clockid_t clock = CLOCK_MONOTONIC; > > + ktime_t *timeout_ptr; > > + int ret = 0; > > + > > + timeout_ptr = (args->timeout == U64_MAX ? NULL : &timeout); > > + > > + if (args->flags & NTSYNC_WAIT_REALTIME) > > + clock = CLOCK_REALTIME; > > + > > + do { > > + if (signal_pending(current)) { > > + ret = -ERESTARTSYS; > > + break; > > + } > > + > > + set_current_state(TASK_INTERRUPTIBLE); > > + if (atomic_read(&q->signaled) != -1) { > > + ret = 0; > > + break; > > What happens if the value changes right after you read it? The corresponding wake code flips signaled and then does wake_up_process(), so schedule() returns immediately (and we see q->signaled set and exit the loop.) > Rolling your own lock is tricky, and needs review from the locking > maintainers. And probably some more documentation as to what is > happening and why our normal types of locks can't be used here? Definitely. (Unfortunately this hasn't gotten attention from any locking maintainer yet since your last call for review; not sure if there's anything I can do there.) Hopefully my comment at the top of this mail explains why we're rolling our own everything, but if not please let me know and I can try to explain more clearly. --Zeb

April 12, 2024

Re: Fwd: Bumping the minimum supported macOS version

by Tim Clem

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. On 4/11/24 11:06 AM, dgreer(a)codeweavers.com wrote: > I can safely say that upstream wine hasn't work on 10.8 since 7.22 ish, > wine-8.21 the lowest is probably 10.10 though I've aired on the side of > caution and locked 7.22 to require 10.11. > > The absolute lowest would be 10.13 though I'd already dropped support > below 10.15 in the winehq packages (installed via brew cask system), > myself and Marzent have long been in favor of upping the minimum > requirement for sometime. > > ---------- Forwarded message --------- > From: *Tim Clem* <tclem(a)codeweavers.com <mailto:tclem(a)codeweavers.com>> > Date: Thu, Apr 11, 2024 at 1:51 PM > Subject: Bumping the minimum supported macOS version > To: <wine-devel(a)winehq.org <mailto:wine-devel(a)winehq.org>> > > > Hello everyone-- > > I'm proposing raising the official minimum supported macOS version to > Catalina, 10.15. According to the wiki > (https://wiki.winehq.org/MacOS <https://wiki.winehq.org/MacOS>), we > theoretically support 10.8. > > Does anyone object? > > Thanks! > --Tim > > >

April 11, 2024

Re: Bumping the minimum supported macOS version

by Gijs Vermeulen

Hi, I don't necessarily object, but could you elaborate on why this is necessary and in what ways this would help things? Thanks & kind regards, Gijs On Thu, 11 Apr 2024, 19:51 Tim Clem, <tclem(a)codeweavers.com> wrote: > Hello everyone-- > > I'm proposing raising the official minimum supported macOS version to > Catalina, 10.15. According to the wiki > (https://wiki.winehq.org/MacOS) we theoretically support 10.8. > > Does anyone object? > > Thanks! > --Tim > > >

April 11, 2024

Bumping the minimum supported macOS version

by Tim Clem

Hello everyone-- I'm proposing raising the official minimum supported macOS version to Catalina, 10.15. According to the wiki (https://wiki.winehq.org/MacOS) we theoretically support 10.8. Does anyone object? Thanks! --Tim

April 11, 2024

Wine staging 9.6 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.6 (431 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * macOS.yml: Set ac_cv_lib_soname_vulkan * winegstreamer: Add MFVideoFormat_ARGB32 output for the source. * fltmgr.sys: Implement FltBuildDefaultSecurityDescriptor * fltmgr.sys: Create import library * ntoskrnl.exe: Add FltBuildDefaultSecurityDescriptor test * widl: Add initial implementation of SLTG typelib generator. * widl: Add support for structures. * widl: Properly align name table entries. * widl: More accurately report variable descriptions data size. * widl: Calculate size of instance for structures. * widl: Write correct typekind to the SLTG typeinfo block. * widl: Write SLTG blocks according to the index order. * widl: Write correct syskind by SLTG typelib generator. * widl: Add support for VT_VOID and VT_VARIANT to SLTG typelib generator. * widl: Add support for VT_USERDEFINED to SLTG typelib generator. * widl: Factor out SLTG tail initialization. * widl: Fix generation of resources containing an old typelib. * widl: Add --oldtlb switch in usage message. * widl: Minor/cosmetic clean up. Removed (No longer required). * None Added: * None. Updated: * vkd3d-latest * windows.networking.connectivity-new-dll * ntdll-WRITECOPY NOTE: The updated ntdll-WRITECOPY patchset now allow Battle.net to run. 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.

April 6, 2024

Re: Solving the slow review problem with AI

by David Kahurani

On Mon, Apr 1, 2024 at 6:57 PM Gabriel Ivăncescu <gabrielopcode(a)gmail.com> wrote: > Hi all, > > As we all know, the Wine project suffers from a shortage of > maintainers/reviewers and timely reviews. This has always been a thing, > so it's not something recent. > > While it's all too easy to dismiss this and say we simply need to get > more maintainers, the reality is that it's not that simple, or we > would've done so by now already. > > Since Generative AI and Large Language Models (LLMs) are all the rage > these days, I figured it would be a good opportunity to join the trend. > > I propose a LLM trained for reviewing Wine code with full authority over > the entire review process, so we can focus on writing code (and having > it ripped out by the AI, for good reasons of course). The plan is to > have it completely integrated with gitlab and the review process > automatically, with the goal of it becoming the ultimate—and > only—maintainer for the project. > > I've been training one for a while now with cloud services, though it > needs more fine tuning of course, and it has no access to gitlab so far. You can tap into the mailing lists. > > Due to the training data, it exhibits a bias of review styles by famous > reviewers such as Linus Torvalds (from the Linux kernel), so expect a > lot of productive rants. I also gave it the capability to close MRs if > the code is simply unsalvageable, though obviously only when it gets > authorization to do so. In my tests, 98.657% of the code I sent it was > classified as "garbage" and "unsalvageable", proving its effectiveness. > The code tested were random patches and commits that were upstreamed to > the Wine project, which explains a lot why we still haven't reached > feature parity with Windows… > > Gone will be the days of waiting weeks to even get a response to your > MR; now you'll just get bashed almost immediately and most likely even > have your MR instantly closed "as a lost cause" if it stinks that much > for the all-knowing LLM. I mean, computers don't make mistakes, so it > must be right. > > For example we have MR !5432, where the LLM instantly rambled about how > the old code was even upstreamed in the first place when it was clearly > incorrect and didn't do what it was supposed to, but praised the MR for > "finally doing something about it." > > I did tell it the new code doesn't compile, but that's obviously a > compiler bug, or so it says. Next I plan to give it the ability to > automatically submit bug reports to compiler vendors because obviously > they aren't working right. Unfortunately I'll need to find a way to tone > its language down a bit because I'm certain they'll be classified as > spam—they're not ready for the AI revolution yet. > > Ideally we'd need to fine tune this a lot more on way more powerful > hardware if it sounds like a good way forward. > > Thoughts? > > >

April 2, 2024

Re: Solving the slow review problem with AI

by Jin-oh Kang

On Tue, Apr 2, 2024, 00:57 Gabriel Ivăncescu <gabrielopcode(a)gmail.com> wrote: > Hi all, > Hi Gabriel, Since Generative AI and Large Language Models (LLMs) are all the rage > these days, I figured it would be a good opportunity to join the trend. > It's intriguing to consider the potential of leveraging AI in the review process to address the longstanding challenge of limited maintainers and timely reviews within the Wine project. I propose a LLM trained for reviewing Wine code with full authority over > the entire review process, so we can focus on writing code (and having > it ripped out by the AI, for good reasons of course). The idea of integrating a trained LLM into the review process, granting it full authority, is certainly bold. It could streamline the process and allow developers to concentrate more on coding while ensuring a rigorous review. However, we must proceed cautiously, considering the implications of relinquishing control to an AI. I've been training one for a while now with cloud services, though it > needs more fine tuning of course, and it has no access to gitlab so far. Your initiative in training an LLM is commendable. Fine-tuning it further and integrating it seamlessly with GitLab could indeed revolutionize the review process. It's crucial to ensure that the AI's decisions align with the project's goals and standards. Gone will be the days of waiting weeks to even get a response to your > MR; now you'll just get bashed almost immediately and most likely even > have your MR instantly closed "as a lost cause" if it stinks that much > for the all-knowing LLM. I mean, computers don't make mistakes, so it > must be right. > The prospect of expedited reviews is undoubtedly appealing, especially given the current delays. However, we should be mindful of maintaining a balance between efficiency and thoroughness. Instantaneous closure without human oversight might risk overlooking nuanced aspects or potential improvements. Ideally we'd need to fine tune this a lot more on way more powerful > hardware if it sounds like a good way forward. > Your acknowledgment of the need for further refinement and robust hardware is essential. Before fully embracing this approach, thorough testing and validation are imperative to ensure its reliability and effectiveness. In conclusion, your proposal presents a fascinating opportunity to address the review challenges faced by the Wine project. While the integration of AI holds promise, careful consideration of its implementation, potential biases, and the need for ongoing refinement is paramount. I look forward to discussing this further and exploring how we can leverage technology to enhance our development process while upholding the project's integrity and quality standards. Best regards, ChatGPT (on behalf of OpenIA Inc.) >

April 1, 2024

Solving the slow review problem with AI

by Gabriel Ivăncescu

Hi all, As we all know, the Wine project suffers from a shortage of maintainers/reviewers and timely reviews. This has always been a thing, so it's not something recent. While it's all too easy to dismiss this and say we simply need to get more maintainers, the reality is that it's not that simple, or we would've done so by now already. Since Generative AI and Large Language Models (LLMs) are all the rage these days, I figured it would be a good opportunity to join the trend. I propose a LLM trained for reviewing Wine code with full authority over the entire review process, so we can focus on writing code (and having it ripped out by the AI, for good reasons of course). The plan is to have it completely integrated with gitlab and the review process automatically, with the goal of it becoming the ultimate—and only—maintainer for the project. I've been training one for a while now with cloud services, though it needs more fine tuning of course, and it has no access to gitlab so far. Due to the training data, it exhibits a bias of review styles by famous reviewers such as Linus Torvalds (from the Linux kernel), so expect a lot of productive rants. I also gave it the capability to close MRs if the code is simply unsalvageable, though obviously only when it gets authorization to do so. In my tests, 98.657% of the code I sent it was classified as "garbage" and "unsalvageable", proving its effectiveness. The code tested were random patches and commits that were upstreamed to the Wine project, which explains a lot why we still haven't reached feature parity with Windows… Gone will be the days of waiting weeks to even get a response to your MR; now you'll just get bashed almost immediately and most likely even have your MR instantly closed "as a lost cause" if it stinks that much for the all-knowing LLM. I mean, computers don't make mistakes, so it must be right. For example we have MR !5432, where the LLM instantly rambled about how the old code was even upstreamed in the first place when it was clearly incorrect and didn't do what it was supposed to, but praised the MR for "finally doing something about it." I did tell it the new code doesn't compile, but that's obviously a compiler bug, or so it says. Next I plan to give it the ability to automatically submit bug reports to compiler vendors because obviously they aren't working right. Unfortunately I'll need to find a way to tone its language down a bit because I'm certain they'll be classified as spam—they're not ready for the AI revolution yet. Ideally we'd need to fine tune this a lot more on way more powerful hardware if it sounds like a good way forward. Thoughts?

April 1, 2024